Bug 91594 – Form state restore: Classify form control sates by owners in internal and serialized representations

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Kent Tamura]

Form state restore: Classify form control sates by owners in internal and serialized representations

[Kent Tamura]

Created attachment 152954 [details]
Patch

[WebKit Review Bot]

Comment on attachment 152954 [details]
Patch

Attachment 152954 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13282421

New failing tests:
fast/forms/state-restore-skip-stateless.html
fast/forms/state-restore-per-form.html

[WebKit Review Bot]

Created attachment 152958 [details]
Archive of layout-test-results from gce-cr-linux-05

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Kent Tamura]

(In reply to comment #2)
> New failing tests:
> fast/forms/state-restore-skip-stateless.html
> fast/forms/state-restore-per-form.html

Ah, the order of states depends on the iteration order of HashMap. We can't assume it's stable. I'll update the tests.

[Kent Tamura]

Created attachment 152963 [details]
Patch 2

Remove stateVector dump

[Hajime Morrita]

Comment on attachment 152963 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=152963&action=review

Another possible approach might be to define a per-key values class and use Set instead if Map.
The advantage of that serialization is more self-contained.
This is just idea though.

> Source/WebCore/html/FormController.cpp:225
> +    stateVector.append(String::number(m_controlStateCount));

We can just count this here instead of maintaining m_controlStateCount.
It's just another loop after all...

[Kent Tamura]

Comment on attachment 152963 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=152963&action=review

Thanks!


>> Source/WebCore/html/FormController.cpp:225
>> +    stateVector.append(String::number(m_controlStateCount));
> 
> We can just count this here instead of maintaining m_controlStateCount.
> It's just another loop after all...

Yes, we can. I had that idea and the m_controlStateCount idea, 
and applied m_controlStateCount because of simplicity.

[WebKit Review Bot]

Comment on attachment 152963 [details]
Patch 2

Clearing flags on attachment: 152963

Committed r123066: <http://trac.webkit.org/changeset/123066>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.