- Revision
- 121145
- Author
- [email protected]
- Date
- 2012-06-25 02:43:07 -0700 (Mon, 25 Jun 2012)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (121144 => 121145)
--- trunk/LayoutTests/ChangeLog 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/LayoutTests/ChangeLog 2012-06-25 09:43:07 UTC (rev 121145)
@@ -1,3 +1,15 @@
+2012-06-25 Kent Tamura <[email protected]>
+
+ 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ó <[email protected]>
[Qt] Unreviewed gardening, skip new failing tests.
Modified: trunk/LayoutTests/fast/forms/state-restore-broken-state-expected.txt (121144 => 121145)
--- trunk/LayoutTests/fast/forms/state-restore-broken-state-expected.txt 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/LayoutTests/fast/forms/state-restore-broken-state-expected.txt 2012-06-25 09:43:07 UTC (rev 121145)
@@ -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".
Added: trunk/LayoutTests/fast/forms/state-restore-various-values-expected.txt (0 => 121145)
--- trunk/LayoutTests/fast/forms/state-restore-various-values-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/state-restore-various-values-expected.txt 2012-06-25 09:43:07 UTC (rev 121145)
@@ -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
+
+
Added: trunk/LayoutTests/fast/forms/state-restore-various-values.html (0 => 121145)
--- trunk/LayoutTests/fast/forms/state-restore-various-values.html (rev 0)
+++ trunk/LayoutTests/fast/forms/state-restore-various-values.html 2012-06-25 09:43:07 UTC (rev 121145)
@@ -0,0 +1,70 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<p>Test if special characters are correctly restored.</p>
+<div id="console"></div>
+
+<input id="emptyOnFirstVisit">
+<div id="parent">
+<form action="" 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">&�
,</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=""
+</body>
Modified: trunk/Source/WTF/ChangeLog (121144 => 121145)
--- trunk/Source/WTF/ChangeLog 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/Source/WTF/ChangeLog 2012-06-25 09:43:07 UTC (rev 121145)
@@ -1,3 +1,15 @@
+2012-06-25 Kent Tamura <[email protected]>
+
+ 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 <[email protected]>
Remove USE(CHROMIUM_NET) because it is unused
Modified: trunk/Source/WTF/wtf/text/StringBuilder.h (121144 => 121145)
--- trunk/Source/WTF/wtf/text/StringBuilder.h 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/Source/WTF/wtf/text/StringBuilder.h 2012-06-25 09:43:07 UTC (rev 121145)
@@ -130,6 +130,21 @@
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();
Modified: trunk/Source/WebCore/ChangeLog (121144 => 121145)
--- trunk/Source/WebCore/ChangeLog 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/Source/WebCore/ChangeLog 2012-06-25 09:43:07 UTC (rev 121145)
@@ -1,3 +1,49 @@
+2012-06-25 Kent Tamura <[email protected]>
+
+ 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 <[email protected]>
Web Inspector: add external resources size to the native memory diagram
Modified: trunk/Source/WebCore/html/FormController.cpp (121144 => 121145)
--- trunk/Source/WebCore/html/FormController.cpp 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/Source/WebCore/html/FormController.cpp 2012-06-25 09:43:07 UTC (rev 121145)
@@ -22,6 +22,7 @@
#include "FormController.h"
#include "HTMLFormControlElementWithState.h"
+#include <wtf/text/StringBuilder.h>
namespace WebCore {
@@ -30,38 +31,61 @@
// ----------------------------------------------------------------------------
// Serilized form of FormControlState:
-// (',' means strings around it are separated in stateVector.)
//
// 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());
- 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)
- 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();
- if (index + valueSize > stateVector.size())
+ if (escaped[0] != ',')
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);
- 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;
}
@@ -81,14 +105,14 @@
// 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;
- 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();
@@ -98,7 +122,7 @@
continue;
stateVector.append(elementWithState->name().string());
stateVector.append(elementWithState->formControlType().string());
- elementWithState->saveFormControlState().serializeTo(stateVector);
+ stateVector.append(elementWithState->saveFormControlState().serialize());
}
return stateVector;
}
@@ -113,14 +137,15 @@
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;
- 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;
@@ -134,8 +159,6 @@
m_stateForNewFormElements.set(key, stateList);
}
}
- if (i != stateVector.size())
- m_stateForNewFormElements.clear();
}
bool FormController::hasStateForNewFormElements() const
Modified: trunk/Source/WebCore/html/FormController.h (121144 => 121145)
--- trunk/Source/WebCore/html/FormController.h 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/Source/WebCore/html/FormController.h 2012-06-25 09:43:07 UTC (rev 121145)
@@ -78,7 +78,7 @@
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&);
@@ -86,7 +86,7 @@
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 };
Modified: trunk/Source/WebCore/html/shadow/CalendarPickerElement.cpp (121144 => 121145)
--- trunk/Source/WebCore/html/shadow/CalendarPickerElement.cpp 2012-06-25 09:26:50 UTC (rev 121144)
+++ trunk/Source/WebCore/html/shadow/CalendarPickerElement.cpp 2012-06-25 09:43:07 UTC (rev 121145)
@@ -151,12 +151,7 @@
{
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);
}
