Bug 91594 - Form state restore: Classify form control sates by owners in internal and serialized representations
Summary: Form state restore: Classify form control sates by owners in internal and ser...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks: 91209
  Show dependency tree / graph
 
Reported: 2012-07-18 00:16 PDT by Kent Tamura
Modified: 2012-07-18 19:49 PDT (History)
3 users (show)

See Also:

Attachments
Patch (23.40 KB, patch)
2012-07-18 00:39 PDT, Kent Tamura 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-05 (316.85 KB, application/zip)
2012-07-18 01:14 PDT, WebKit Review Bot 		no flags Details
Patch 2 (27.20 KB, patch)
2012-07-18 01:57 PDT, Kent Tamura 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-07-18 00:16:52 PDT 
Form state restore: Classify form control sates by owners in internal and serialized representations
Comment 1 Kent Tamura 2012-07-18 00:39:25 PDT 
Created attachment 152954 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-18 01:14:20 PDT 
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
Comment 3 WebKit Review Bot 2012-07-18 01:14:23 PDT 
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
Comment 4 Kent Tamura 2012-07-18 01:32:20 PDT 
(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.
Comment 5 Kent Tamura 2012-07-18 01:57:53 PDT 
Created attachment 152963 [details]
Patch 2

Remove stateVector dump
Comment 6 Hajime Morrita 2012-07-18 19:07:44 PDT 
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...
Comment 7 Kent Tamura 2012-07-18 19:13:29 PDT 
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.
Comment 8 WebKit Review Bot 2012-07-18 19:49:04 PDT 
Comment on attachment 152963 [details]
Patch 2

Clearing flags on attachment: 152963

Committed r123066: <http://trac.webkit.org/changeset/123066>
Comment 9 WebKit Review Bot 2012-07-18 19:49:09 PDT 
All reviewed patches have been landed.  Closing bug.