In order to update routing ids for loading requests when iframe is reparented between WebFrames, we need to add a notification to WebFrameClient to tell the WebFrame it now owns a specified resource in progress.
Created attachment 103818 [details] Patch
This patch won't compile w/o previous one from bug 66165 since it uses a new RessourceLoader* parameter of FrameLoaderClient::transferLoadingResourceFromPage
Comment on attachment 103818 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103818&action=review > Source/WebKit/chromium/public/WebFrameClient.h:291 > + virtual void adoptLoadingResource(WebURLLoader*) { } Perhaps this should be adoptURLLoader... or, maybe even better: didAdoptURLLoader. The second form suggests that this is informative. The client does not necessarily need to do anything here unless it has special needs. Chromium has special needs. DRT does not. > Source/WebKit/chromium/src/ResourceHandle.cpp:83 > + static WebURLLoader* WebURLLoaderFromResourceHandle(ResourceHandle* handle) { did you really mean for this to be a static method on ResourceHandleInternal?
Created attachment 103821 [details] Updated patch Renamed method to didAdoptURLLoader. As for static method - I hope there is a better way, I'm using the fact that ResourceHandleInternal is a 'friend' to ResourceHandle, to pull the 'd' member. We currently don't have access to it otherwise. Do I miss something?
Created attachment 103822 [details] Updated ChanegLog
Attachment 103821 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/src/ResourceHandle.cpp:83: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 103822 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/src/ResourceHandle.cpp:83: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 103825 [details] Fixed style error.
Comment on attachment 103825 [details] Fixed style error. View in context: https://bugs.webkit.org/attachment.cgi?id=103825&action=review > Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp:1463 > + WebURLLoader* webURLLoader = WebURLLoaderFromResourceHandle(handle); The thing that confuses me is that at this callsite, you are not doing ResourceHandleInternal::WebURLLoaderFromResourceHandle. so, there must be a missing file that defines the global WebURLLoaderFromResourceHandle function?
(In reply to comment #9) > (From update of attachment 103825 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=103825&action=review > > > Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp:1463 > > + WebURLLoader* webURLLoader = WebURLLoaderFromResourceHandle(handle); > > The thing that confuses me is that at this callsite, you are not doing ResourceHandleInternal::WebURLLoaderFromResourceHandle. so, there must be a missing file that defines the global WebURLLoaderFromResourceHandle function? Very true. It won't compile. My bad. How do we crack open ResourceHandle?
Comment on attachment 103825 [details] Fixed style error. Attachment 103825 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9378018
Comment on attachment 103825 [details] Fixed style error. Attachment 103825 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9357844
Created attachment 103850 [details] Patch As discussed offline, created ResourceHandleInternal.h in chromium/src. The impl of the class still lives in ResourceHandle.cpp, I can pull it out into separate file if desired. Data members of ResourceHandleInternal are made private, needed accessors are added. The complete fix consists of WebKit patches for bug 66165 and this one, and also Chromium side here: http://codereview.chromium.org/7647003/
Attachment 103850 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/src/ResourceHandleInternal.h:83: set_owner is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebKit/chromium/src/ResourceHandleInternal.h:86: set_client is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 103850 [details] Patch Attachment 103850 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9380061
Comment on attachment 103850 [details] Patch Attachment 103850 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9378131
Comment on attachment 103850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103850&action=review > Source/WebKit/chromium/src/ResourceHandleInternal.h:99 > + ConnectionState m_state; is this still used for anything? i don't see any way for this to change values.
Comment on attachment 103850 [details] Patch Will have updated patch in a moment.
Created attachment 103938 [details] Fixed style issue As for m_state member, it's used internally in implementation of Chromium's ResourceHandleInternal to track the states. I can see it changing values and then asserting on them in WebKit/chromium/src/ResourceHandle.cpp
Comment on attachment 103938 [details] Fixed style issue Attachment 103938 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9381846
Comment on attachment 103938 [details] Fixed style issue Attachment 103938 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9394617
Comment on attachment 103938 [details] Fixed style issue View in context: https://bugs.webkit.org/attachment.cgi?id=103938&action=review > Source/WebKit/chromium/public/WebFrameClient.h:291 > + // when iframe is transferred between windows while it is loading something. "This happens when an iframe, that is loading a subresource, is transferred between windows."
Updated and landed manually: http://trac.webkit.org/changeset/93076