Summary: | Chromium: Update resource tracking when moving a frame between documents | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jenn Braithwaite <jennb> | ||||||||
Component: | Page Loading | Assignee: | Jenn Braithwaite <jennb> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, commit-queue, dimich, fishd | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | 44713 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Jenn Braithwaite
2010-10-26 12:33:14 PDT
Created attachment 72121 [details]
patch
Comment on attachment 72121 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=72121&action=review > WebKit/chromium/src/FrameLoaderClientImpl.cpp:1394 > + oldWebFrame->client()->removeIdentifierForRequest(oldWebFrame, identifier); It looks like we are passing here wrong webFrame... Do we really need this parameter? Created attachment 72261 [details]
updated patch to remove unnecessary frame parameter
Needs fishd for WebKit API review. Created attachment 72526 [details]
updated patch - use map.contains()
Comment on attachment 72526 [details]
updated patch - use map.contains()
API change looks good to me. Is it possible to write a layout test for this?
(In reply to comment #6) > (From update of attachment 72526 [details]) > API change looks good to me. Is it possible to write a layout test for this? The layout test is in the patch submitted with bug 44713. Both that test (fast/frames/iframe-reparenting-fail-load.html) and fast/frames/iframe-reparenting-adopt-node.html cover this change. Comment on attachment 72526 [details]
updated patch - use map.contains()
r=me
Comment on attachment 72526 [details] updated patch - use map.contains() Clearing flags on attachment: 72526 Committed r71297: <http://trac.webkit.org/changeset/71297> All reviewed patches have been landed. Closing bug. |