Bug 47002 - Make resource identifiers unique across pages
Summary: Make resource identifiers unique across pages
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Jenn Braithwaite
Depends on:
Blocks: 44713
  Show dependency treegraph
Reported: 2010-10-01 11:41 PDT by Jenn Braithwaite
Modified: 2010-10-13 02:03 PDT (History)
3 users (show)

See Also:

patch (6.57 KB, patch)
2010-10-05 12:04 PDT, Jenn Braithwaite
dimich: review-
Details | Formatted Diff | Diff
Updated patch. (7.28 KB, patch)
2010-10-11 13:30 PDT, Jenn Braithwaite
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jenn Braithwaite 2010-10-01 11:41:57 PDT
In order to fix bug 44713, resource identifiers need to be unique across pages so that the resource id can be used in more than one Page/WebView.  Making the ids unique is cleaner than changing the ids before using them in another object on a different page.

Resource identifiers are generated from the Page's ProgressTracker.  There are other uses of ProgressTracker::createUniqueIdentifier, but they are not dependent on the ids being only unique within a page, so this change should be safe.
Comment 1 Jenn Braithwaite 2010-10-05 12:04:10 PDT
Created attachment 69823 [details]
Comment 2 Dmitry Titov 2010-10-06 11:20:57 PDT
Comment on attachment 69823 [details]

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

Looks great, I understand the actual ASSERTS will come as a close next patch since they rely on more code, so this adds a test ahead of code, which is ok.
r- since the test can be improved.

> LayoutTests/fast/frames/iframe-reparenting-id-collision.html:42
> +    w1.addEventListener("load", windowLoaded, false);

It could be more reliable to make this test to wait until both windows are loaded, but at the same time to ensure that their inner iframes are still loading.
It can be accomplished by making this test a http test and hitting the 'slow loading' server script from iframes, while registering onload handler for both windows. This will guarantee that the test is free from timing issues.
Comment 3 Jenn Braithwaite 2010-10-11 13:30:57 PDT
Created attachment 70463 [details]
Updated patch.

Replaced test case with an http test.
Comment 4 Dmitry Titov 2010-10-13 01:42:09 PDT
Comment on attachment 70463 [details]
Updated patch.

Comment 5 WebKit Commit Bot 2010-10-13 02:03:54 PDT
Comment on attachment 70463 [details]
Updated patch.

Clearing flags on attachment: 70463

Committed r69643: <http://trac.webkit.org/changeset/69643>
Comment 6 WebKit Commit Bot 2010-10-13 02:03:59 PDT
All reviewed patches have been landed.  Closing bug.