RESOLVED FIXED 81590
Remove support for "magic" iframe 
https://bugs.webkit.org/show_bug.cgi?id=81590
Summary Remove support for "magic" iframe
Adam Barth
Reported 2012-03-19 16:31:28 PDT
Remove support for "magic" iframe
Attachments
Patch (47.97 KB, patch)
2012-03-19 16:36 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (77.80 KB, patch)
2012-03-19 21:06 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (80.53 KB, patch)
2012-03-19 21:13 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2012-03-19 16:36:59 PDT
Created attachment 132713 [details] Patch
Abhishek Arya
Comment 2 2012-03-19 16:39:46 PDT
This will be SOSO awesome if it lands before next Tuesday m19 branch point :)!
WebKit Review Bot
Comment 3 2012-03-19 16:48:28 PDT
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
WebKit Review Bot
Comment 4 2012-03-19 16:48:50 PDT
Attachment 132713 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/loader/FrameLoaderClient.h:259: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/loader/FrameLoaderClient.h:259: The parameter name "ownerElement" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dmitry Titov
Comment 5 2012-03-19 17:00:40 PDT
Comment on attachment 132713 [details] Patch Thanks a lot Adam for doing this patch! WebFrameLoader::didAdoptURLLoader(..) should also be removed. There are layout tests as well, but they should fail on EWS and their names are very obviously-magic-iframe-related.
Adam Barth
Comment 6 2012-03-19 17:24:31 PDT
Comment on attachment 132713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132713&action=review > Source/WebKit/chromium/public/WebFrameClient.h:-289 > - // This frame adopted the resource that is being loaded. This happens when > - // an iframe, that is loading a subresource, is transferred between windows. > - virtual void didAdoptURLLoader(WebURLLoader*) { } Yeah, we need to remove the implementation of this function on the Chromium side.
Build Bot
Comment 7 2012-03-19 21:02:32 PDT
Comment on attachment 132713 [details] Patch Attachment 132713 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/11994070
Darin Fisher (:fishd, Google)
Comment 8 2012-03-19 21:05:31 PDT
Comment on attachment 132713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132713&action=review >> Source/WebKit/chromium/public/WebFrameClient.h:-289 >> - virtual void didAdoptURLLoader(WebURLLoader*) { } > > Yeah, we need to remove the implementation of this function on the Chromium side. LGTM for nuking this method.
Adam Barth
Comment 9 2012-03-19 21:06:01 PDT
Created attachment 132751 [details] Patch
WebKit Review Bot
Comment 10 2012-03-19 21:09:59 PDT
Attachment 132751 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/loader/FrameLoaderClient.h:259: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/loader/FrameLoaderClient.h:259: The parameter name "ownerElement" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Adam Barth
Comment 11 2012-03-19 21:13:39 PDT
Created attachment 132752 [details] Patch
WebKit Review Bot
Comment 12 2012-03-19 21:17:56 PDT
Attachment 132752 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/loader/FrameLoaderClient.h:259: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/loader/FrameLoaderClient.h:259: The parameter name "ownerElement" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Eric Seidel (no email)
Comment 13 2012-03-19 21:27:07 PDT
Comment on attachment 132752 [details] Patch Huge win! Thanks!
WebKit Review Bot
Comment 14 2012-03-19 22:58:57 PDT
Comment on attachment 132752 [details] Patch Clearing flags on attachment: 132752 Committed r111361: <http://trac.webkit.org/changeset/111361>
WebKit Review Bot
Comment 15 2012-03-19 22:59:03 PDT
All reviewed patches have been landed. Closing bug.
Jessie Berlin
Comment 16 2012-03-20 10:12:21 PDT
(In reply to comment #14) > (From update of attachment 132752 [details]) > Clearing flags on attachment: 132752 > > Committed r111361: <http://trac.webkit.org/changeset/111361> It appears that you removed all the fast/frames/iframe-reparenting* tests except iframe-reparenting-unique-name.html. Was it intentional to leave that one in?
Adam Barth
Comment 17 2012-03-20 10:44:21 PDT
> Was it intentional to leave that one in? Yeah, that test still seems to make sense without the feature.
Note You need to log in before you can comment on or make changes to this bug.