Change the serialization format of form control state to make the code simple
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Jun 2012 09:43:07 +0000 (09:43 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Jun 2012 09:43:07 +0000 (09:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=89847

Reviewed by Hajime Morita.

Source/WebCore:

We used multiple strings to represent state of single form control. It
made the code complex. We change the serialization format so that one
CSV string represents state.

Examples in the old format:
    "0"
    "1", "value"
    "3", "value1", "value2,value2", "value3"

Examples in the new format:
    ""
    ",value"
    ",value1,value2\,value2,value3"

Test: fast/forms/state-restore-various-values.html

* html/FormController.cpp:
(WebCore::FormControlState::serialize):
Generate comma-separated string.
',' in a value is serialized as "\,".
We changed the signature because we don't need the out-argument.
(WebCore::FormControlState::deserialize):
Parses the input comma-separated string.
We changed the signature because we don't need multiple input strings.
(formStateSignature):
Bump up the version because of the representation change.
(WebCore::FormController::formElementsState):
The new serialized format occupies just one string for one control.
- Expected size is now 3n+1.
- Use FormControlState::serialize().
(WebCore::FormController::setStateForNewFormElements):
The new serialized format occupies just one string for one control.
So we can check the vector size before the iteration.
* html/FormController.h:
(FormControlState): Change the function signatures.

* html/shadow/CalendarPickerElement.cpp:
(WebCore::addJavaScriptString): Use StringBuilder::appendEscaped().

Source/WTF:

* wtf/text/StringBuilder.h:
(WTF::StringBuilder::appendEscaped): Added. This function adds the
escaped form of the input string. e.g. if stiring="foo,bar" escape='\'
special=',', the appended string is foo\,bar.

LayoutTests:

* fast/forms/state-restore-broken-state-expected.txt:
Apply the serialization format change.
* fast/forms/state-restore-various-values-expected.txt: Added.
* fast/forms/state-restore-various-values.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@121145 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/forms/state-restore-broken-state-expected.txt
LayoutTests/fast/forms/state-restore-various-values-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/state-restore-various-values.html [new file with mode: 0644]
Source/WTF/ChangeLog
Source/WTF/wtf/text/StringBuilder.h
Source/WebCore/ChangeLog
Source/WebCore/html/FormController.cpp
Source/WebCore/html/FormController.h
Source/WebCore/html/shadow/CalendarPickerElement.cpp

index e069d0b103846de443fa3ae52e91c4d8b3ae3b6c..eca8c979d0999150d5b32451ed17110adb7c7be7 100644 (file)
@@ -1,3 +1,15 @@
+2012-06-25  Kent Tamura  <tkent@chromium.org>
+
+        Change the serialization format of form control state to make the code simple
+        https://bugs.webkit.org/show_bug.cgi?id=89847
+
+        Reviewed by Hajime Morita.
+
+        * fast/forms/state-restore-broken-state-expected.txt:
+        Apply the serialization format change.
+        * fast/forms/state-restore-various-values-expected.txt: Added.
+        * fast/forms/state-restore-various-values.html: Added.
+
 2012-06-24  Kristóf Kosztyó  <kkristof@inf.u-szeged.hu>
 
         [Qt] Unreviewed gardening, skip new failing tests.
 2012-06-24  Kristóf Kosztyó  <kkristof@inf.u-szeged.hu>
 
         [Qt] Unreviewed gardening, skip new failing tests.
index d952e93f6b15560b1029d18d78b05a4a72eac115..25f6dc7714870f15640e230188bcb6e2539219da 100644 (file)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 5: Generated state: [name1,text,1,modified]
+CONSOLE MESSAGE: line 5: Generated state: [name1,text,,modified]
 The value was modified in the first load of state-restore-broken-state-1.html, but it should not be restored because the state-restore-broken-state-2.html breaks the state.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 The value was modified in the first load of state-restore-broken-state-1.html, but it should not be restored because the state-restore-broken-state-2.html breaks the state.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
diff --git a/LayoutTests/fast/forms/state-restore-various-values-expected.txt b/LayoutTests/fast/forms/state-restore-various-values-expected.txt
new file mode 100644 (file)
index 0000000..ad36482
--- /dev/null
@@ -0,0 +1,17 @@
+Test if special characters are correctly restored.
+
+PASS $("opt-01").selected is true
+PASS $("opt-02").selected is true
+PASS $("opt-03").selected is true
+PASS $("opt-04").selected is true
+PASS $("opt-05").selected is true
+PASS $("opt-06").selected is true
+PASS $("opt-07").selected is true
+PASS $("opt-08").selected is true
+PASS $("opt-09").selected is true
+PASS $("opt-10").selected is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
diff --git a/LayoutTests/fast/forms/state-restore-various-values.html b/LayoutTests/fast/forms/state-restore-various-values.html
new file mode 100644 (file)
index 0000000..5e137e5
--- /dev/null
@@ -0,0 +1,70 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../js/resources/js-test-pre.js"></script>
+<script src="resources/common.js"></script>
+</head>
+<body>
+<p>Test if special characters are correctly restored.</p>
+<div id="console"></div>
+
+<input id="emptyOnFirstVisit">
+<div id="parent">
+<form action="data:text/html,&lt;script>history.back()&lt;/script>" id=form1>
+<select id="select1" multiple>
+  <option id="opt-01"></option>
+  <option id="opt-02">,</option>
+  <option id="opt-03">\</option>
+  <option id="opt-04">,a,</option>
+  <option id="opt-05">,\,</option>
+  <option id="opt-06">,\\,</option>
+  <option id="opt-07">\a\</option>
+  <option id="opt-08">\n\</option>
+  <option id="opt-09">\,,\</option>
+  <option id="opt-10">&amp;&#0;&#x0d;&#x0a;,</option>
+</select>
+</form>
+</div>
+
+<script>
+// Note that this test depends on the fact that select options are stored by
+// value strings, not indexes.
+
+jsTestIsAsync = true;
+
+function runTest()
+{
+    var state = document.getElementById('emptyOnFirstVisit');
+    if (!state.value) {
+        // First visit.
+        setTimeout(function() {
+            state.value = 'visited';
+            var options = $('select1').options;
+            for (var i = 0; i < options.length; ++i)
+                options[i].selected = true;
+            $('form1').submit();
+        }, 0);
+    } else {
+        // Went back to this page again, and form state should be restored.
+        shouldBeTrue('$("opt-01").selected');
+        shouldBeTrue('$("opt-02").selected');
+        shouldBeTrue('$("opt-03").selected');
+        shouldBeTrue('$("opt-04").selected');
+        shouldBeTrue('$("opt-05").selected');
+        shouldBeTrue('$("opt-06").selected');
+        shouldBeTrue('$("opt-07").selected');
+        shouldBeTrue('$("opt-08").selected');
+        shouldBeTrue('$("opt-09").selected');
+        shouldBeTrue('$("opt-10").selected');
+    
+        $('parent').innerHTML = '';
+        setTimeout(function() {
+            finishJSTest();
+        }, 0);
+    }
+}
+
+runTest();
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
index be2aaa843499fa6affd6301469535ff79e7a15d9..8a3746769488bbbf2298309ce9efb2ddaf4ec452 100644 (file)
@@ -1,3 +1,15 @@
+2012-06-25  Kent Tamura  <tkent@chromium.org>
+
+        Change the serialization format of form control state to make the code simple
+        https://bugs.webkit.org/show_bug.cgi?id=89847
+
+        Reviewed by Hajime Morita.
+
+        * wtf/text/StringBuilder.h:
+        (WTF::StringBuilder::appendEscaped): Added. This function adds the
+        escaped form of the input string. e.g. if stiring="foo,bar" escape='\'
+        special=',', the appended string is foo\,bar.
+
 2012-06-24  Adam Barth  <abarth@webkit.org>
 
         Remove USE(CHROMIUM_NET) because it is unused
 2012-06-24  Adam Barth  <abarth@webkit.org>
 
         Remove USE(CHROMIUM_NET) because it is unused
index d896d17b1551c4ddabc4daa3a271f19bfdf0c6a0..c73d46372cb54c0c63b3409c8fc142be1a0f23ce 100644 (file)
@@ -130,6 +130,21 @@ public:
         append(static_cast<LChar>(c));
     }
 
         append(static_cast<LChar>(c));
     }
 
+    void appendEscaped(const String& string, UChar escape, UChar special)
+    {
+        if (string.isEmpty())
+            return;
+        unsigned requiredSize = length() + string.length();
+        if (capacity() < requiredSize)
+            reserveCapacity(requiredSize);
+        for (unsigned i = 0; i < string.length(); ++i) {
+            UChar ch = string[i];
+            if (ch == escape || ch == special)
+                append(escape);
+            append(ch);
+        }
+    }
+
     String toString()
     {
         shrinkToFit();
     String toString()
     {
         shrinkToFit();
index c3e694bb174b383926b7fc364ed17c3c72255325..4a4f84dac24dae11d8622d4b263a00549744f012 100644 (file)
@@ -1,3 +1,49 @@
+2012-06-25  Kent Tamura  <tkent@chromium.org>
+
+        Change the serialization format of form control state to make the code simple
+        https://bugs.webkit.org/show_bug.cgi?id=89847
+
+        Reviewed by Hajime Morita.
+
+        We used multiple strings to represent state of single form control. It
+        made the code complex. We change the serialization format so that one
+        CSV string represents state.
+
+        Examples in the old format:
+            "0"
+            "1", "value"
+            "3", "value1", "value2,value2", "value3"
+
+        Examples in the new format:
+            ""
+            ",value"
+            ",value1,value2\,value2,value3"
+
+        Test: fast/forms/state-restore-various-values.html
+
+        * html/FormController.cpp:
+        (WebCore::FormControlState::serialize):
+        Generate comma-separated string.
+        ',' in a value is serialized as "\,".
+        We changed the signature because we don't need the out-argument.
+        (WebCore::FormControlState::deserialize):
+        Parses the input comma-separated string.
+        We changed the signature because we don't need multiple input strings.
+        (formStateSignature):
+        Bump up the version because of the representation change.
+        (WebCore::FormController::formElementsState):
+        The new serialized format occupies just one string for one control.
+        - Expected size is now 3n+1.
+        - Use FormControlState::serialize().
+        (WebCore::FormController::setStateForNewFormElements):
+        The new serialized format occupies just one string for one control.
+        So we can check the vector size before the iteration.
+        * html/FormController.h:
+        (FormControlState): Change the function signatures.
+
+        * html/shadow/CalendarPickerElement.cpp:
+        (WebCore::addJavaScriptString): Use StringBuilder::appendEscaped().
+
 2012-06-22  Yury Semikhatsky  <yurys@chromium.org>
 
         Web Inspector: add external resources size to the native memory diagram
 2012-06-22  Yury Semikhatsky  <yurys@chromium.org>
 
         Web Inspector: add external resources size to the native memory diagram
index 31c776343cb3c6ffbc1f55a2e242647d8694d9c1..84f44ee9416df08e9872b230730f193bd6de8e30 100644 (file)
@@ -22,6 +22,7 @@
 #include "FormController.h"
 
 #include "HTMLFormControlElementWithState.h"
 #include "FormController.h"
 
 #include "HTMLFormControlElementWithState.h"
+#include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
 
 namespace WebCore {
 
@@ -30,38 +31,61 @@ using namespace HTMLNames;
 // ----------------------------------------------------------------------------
 
 // Serilized form of FormControlState:
 // ----------------------------------------------------------------------------
 
 // Serilized form of FormControlState:
-//  (',' means strings around it are separated in stateVector.)
 //
 // SerializedControlState ::= SkipState | RestoreState
 //
 // SerializedControlState ::= SkipState | RestoreState
-// SkipState ::= '0'
-// RestoreState ::= UnsignedNumber, ControlValue+
-// UnsignedNumber ::= [0-9]+
-// ControlValue ::= arbitrary string
-//
-// RestoreState has a sequence of ControlValues. The length of the
-// sequence is represented by UnsignedNumber.
+// SkipState ::= ''
+// RestoreState ::= (',' EscapedValue )+
+// EscapedValue ::= ('\\' | '\,' | [^\,])+
 
 
-void FormControlState::serializeTo(Vector<String>& stateVector) const
+String FormControlState::serialize() const
 {
     ASSERT(!isFailure());
 {
     ASSERT(!isFailure());
-    stateVector.append(String::number(m_values.size()));
+    if (!m_values.size())
+        return emptyString();
+
+    size_t enoughSize = 0;
     for (size_t i = 0; i < m_values.size(); ++i)
     for (size_t i = 0; i < m_values.size(); ++i)
-        stateVector.append(m_values[i].isNull() ? emptyString() : m_values[i]);
+        enoughSize += 1 + m_values[i].length() * 2;
+    StringBuilder builder;
+    builder.reserveCapacity(enoughSize);
+    for (size_t i = 0; i < m_values.size(); ++i) {
+        builder.append(',');
+        builder.appendEscaped(m_values[i], '\\', ',');
+    }
+    return builder.toString();
 }
 
 }
 
-FormControlState FormControlState::deserialize(const Vector<String>& stateVector, size_t& index)
+FormControlState FormControlState::deserialize(const String& escaped)
 {
 {
-    if (index >= stateVector.size())
-        return FormControlState(TypeFailure);
-    size_t valueSize = stateVector[index++].toUInt();
-    if (!valueSize)
+    if (!escaped.length())
         return FormControlState();
         return FormControlState();
-    if (index + valueSize > stateVector.size())
+    if (escaped[0] != ',')
         return FormControlState(TypeFailure);
         return FormControlState(TypeFailure);
+
+    size_t valueSize = 1;
+    for (unsigned i = 1; i < escaped.length(); ++i) {
+        if (escaped[i] == '\\') {
+            if (++i >= escaped.length())
+                return FormControlState(TypeFailure);
+        } else if (escaped[i] == ',')
+            valueSize++;
+    }
+
     FormControlState state;
     state.m_values.reserveCapacity(valueSize);
     FormControlState state;
     state.m_values.reserveCapacity(valueSize);
-    for (size_t i = 0; i < valueSize; ++i)
-        state.append(stateVector[index++]);
+    StringBuilder builder;
+    for (unsigned i = 1; i < escaped.length(); ++i) {
+        if (escaped[i] == '\\') {
+            if (++i >= escaped.length())
+                return FormControlState(TypeFailure);
+            builder.append(escaped[i]);
+        } else if (escaped[i] == ',') {
+            state.append(builder.toString());
+            builder.clear();
+        } else
+            builder.append(escaped[i]);
+    }
+    state.append(builder.toString());
     return state;
 }
 
     return state;
 }
 
@@ -81,14 +105,14 @@ static String formStateSignature()
     // In the legacy version of serialized state, the first item was a name
     // attribute value of a form control. The following string literal should
     // contain some characters which are rarely used for name attribute values.
     // In the legacy version of serialized state, the first item was a name
     // attribute value of a form control. The following string literal should
     // contain some characters which are rarely used for name attribute values.
-    DEFINE_STATIC_LOCAL(String, signature, ("\n\r?% WebKit serialized form state version 3 \n\r=&"));
+    DEFINE_STATIC_LOCAL(String, signature, ("\n\r?% WebKit serialized form state version 4 \n\r=&"));
     return signature;
 }
 
 Vector<String> FormController::formElementsState() const
 {
     Vector<String> stateVector;
     return signature;
 }
 
 Vector<String> FormController::formElementsState() const
 {
     Vector<String> stateVector;
-    stateVector.reserveInitialCapacity(m_formElementsWithState.size() * 4 + 1);
+    stateVector.reserveInitialCapacity(m_formElementsWithState.size() * 3 + 1);
     stateVector.append(formStateSignature());
     typedef FormElementListHashSet::const_iterator Iterator;
     Iterator end = m_formElementsWithState.end();
     stateVector.append(formStateSignature());
     typedef FormElementListHashSet::const_iterator Iterator;
     Iterator end = m_formElementsWithState.end();
@@ -98,7 +122,7 @@ Vector<String> FormController::formElementsState() const
             continue;
         stateVector.append(elementWithState->name().string());
         stateVector.append(elementWithState->formControlType().string());
             continue;
         stateVector.append(elementWithState->name().string());
         stateVector.append(elementWithState->formControlType().string());
-        elementWithState->saveFormControlState().serializeTo(stateVector);
+        stateVector.append(elementWithState->saveFormControlState().serialize());
     }
     return stateVector;
 }
     }
     return stateVector;
 }
@@ -113,14 +137,15 @@ void FormController::setStateForNewFormElements(const Vector<String>& stateVecto
     typedef FormElementStateMap::iterator Iterator;
     m_formElementsWithState.clear();
 
     typedef FormElementStateMap::iterator Iterator;
     m_formElementsWithState.clear();
 
-    size_t i = 0;
-    if (stateVector.size() < 1 || stateVector[i++] != formStateSignature())
+    if (stateVector.size() < 1 || stateVector[0] != formStateSignature())
+        return;
+    if ((stateVector.size() - 1) % 3)
         return;
 
         return;
 
-    while (i + 2 < stateVector.size()) {
-        AtomicString name = stateVector[i++];
-        AtomicString type = stateVector[i++];
-        FormControlState state = FormControlState::deserialize(stateVector, i);
+    for (size_t i = 1; i < stateVector.size(); i += 3) {
+        AtomicString name = stateVector[i];
+        AtomicString type = stateVector[i + 1];
+        FormControlState state = FormControlState::deserialize(stateVector[i + 2]);
         if (type.isEmpty() || type.impl()->find(isNotFormControlTypeCharacter) != notFound || state.isFailure())
             break;
 
         if (type.isEmpty() || type.impl()->find(isNotFormControlTypeCharacter) != notFound || state.isFailure())
             break;
 
@@ -134,8 +159,6 @@ void FormController::setStateForNewFormElements(const Vector<String>& stateVecto
             m_stateForNewFormElements.set(key, stateList);
         }
     }
             m_stateForNewFormElements.set(key, stateList);
         }
     }
-    if (i != stateVector.size())
-        m_stateForNewFormElements.clear();
 }
 
 bool FormController::hasStateForNewFormElements() const
 }
 
 bool FormController::hasStateForNewFormElements() const
index fa5463b8ae376746013afbfc895dafd706d2fe9a..4de51f524b565ce2eaef475b42086f9e8d3ac893 100644 (file)
@@ -78,7 +78,7 @@ class FormControlState {
 public:
     FormControlState() : m_type(TypeSkip) { }
     explicit FormControlState(const String& value) : m_type(TypeRestore) { m_values.append(value); }
 public:
     FormControlState() : m_type(TypeSkip) { }
     explicit FormControlState(const String& value) : m_type(TypeRestore) { m_values.append(value); }
-    static FormControlState deserialize(const Vector<String>& stateVector, size_t& index);
+    static FormControlState deserialize(const String&);
     FormControlState(const FormControlState& another) : m_type(another.m_type), m_values(another.m_values) { }
     FormControlState& operator=(const FormControlState&);
 
     FormControlState(const FormControlState& another) : m_type(another.m_type), m_values(another.m_values) { }
     FormControlState& operator=(const FormControlState&);
 
@@ -86,7 +86,7 @@ public:
     size_t valueSize() const { return m_values.size(); }
     const String& operator[](size_t i) const { return m_values[i]; }
     void append(const String&);
     size_t valueSize() const { return m_values.size(); }
     const String& operator[](size_t i) const { return m_values[i]; }
     void append(const String&);
-    void serializeTo(Vector<String>& stateVector) const;
+    String serialize() const;
 
 private:
     enum Type { TypeSkip, TypeRestore, TypeFailure };
 
 private:
     enum Type { TypeSkip, TypeRestore, TypeFailure };
index 5358ccaf7a3df2e33a03ebf0c88911b0c0882bc5..fe1f64e13e7aca2ac714ae6c3f8e2b2ee00f8075 100644 (file)
@@ -151,12 +151,7 @@ static void addJavaScriptString(const String& str, DocumentWriter& writer)
 {
     addLiteral("\"", writer);
     StringBuilder builder;
 {
     addLiteral("\"", writer);
     StringBuilder builder;
-    builder.reserveCapacity(str.length());
-    for (unsigned i = 0; i < str.length(); ++i) {
-        if (str[i] == '\\' || str[i] == '"')
-            builder.append('\\');
-        builder.append(str[i]);
-    }
+    builder.appendEscaped(str, '\\', '"');
     addString(builder.toString(), writer);
     addLiteral("\"", writer);
 }
     addString(builder.toString(), writer);
     addLiteral("\"", writer);
 }