Bug 144276 - Consolidate most "frame load" arguments into FrameLoadRequest
Summary: Consolidate most "frame load" arguments into FrameLoadRequest
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 144269
  Show dependency treegraph
 
Reported: 2015-04-27 13:55 PDT by Brady Eidson
Modified: 2016-03-23 14:46 PDT (History)
5 users (show)

See Also:


Attachments
Patch for EWS (35.14 KB, patch)
2015-04-27 16:55 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (611.77 KB, application/zip)
2015-04-27 17:47 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (618.13 KB, application/zip)
2015-04-27 18:09 PDT, Build Bot
no flags Details
Patch EWS (35.70 KB, patch)
2015-04-28 09:42 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch for EWS (34.50 KB, patch)
2015-04-28 10:38 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v1 (39.86 KB, patch)
2015-04-28 11:30 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-04-27 13:55:09 PDT
Consolidate most "frame load" arguments into FrameLoadRequest

Right now, the various "load a frame" functions in FrameLoader pass around a gobbledygook of distinct arguments that really should be inside FrameLoadRequest itself.
Comment 1 Brady Eidson 2015-04-27 15:56:35 PDT
There's a lot of work later to continue to whittle away the various "loadRequest" methods in FrameLoader, including minimizing the number of them that take all of the superfluous FrameLoadRequest arguments.

But I'll be presenting a "good start" that consolidates down at least one of the entry points into FrameLoader. (blocking work on https://bugs.webkit.org/show_bug.cgi?id=144269)
Comment 2 Brady Eidson 2015-04-27 16:55:23 PDT
Created attachment 251786 [details]
Patch for EWS
Comment 3 WebKit Commit Bot 2015-04-27 16:56:41 PDT
Attachment 251786 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/FrameLoadRequest.h:112:  Extra space between LockHistory and m_lockHistory  [whitespace/declaration] [3]
ERROR: Source/WebCore/loader/FrameLoadRequest.h:113:  Extra space between LockBackForwardList and m_lockBackForwardList  [whitespace/declaration] [3]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Build Bot 2015-04-27 17:47:41 PDT
Comment on attachment 251786 [details]
Patch for EWS

Attachment 251786 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5218143597232128

New failing tests:
fast/dom/null-document-location-assign-crash.html
http/tests/security/xss-DENIED-synchronous-frame-load-in-javascript-url.html
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-window-open.html
fast/dom/null-document-location-href-put-crash.html
fast/dom/null-document-location-put-crash.html
fast/dom/null-document-location-replace-crash.html
http/tests/security/postMessage/javascript-page-still-sends-origin.html
fast/loader/javascript-url-encoding-2.html
http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html
fast/dom/null-document-window-open-crash.html
http/tests/security/aboutBlank/window-open-self-about-blank.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-window-open.html
Comment 5 Build Bot 2015-04-27 17:47:43 PDT
Created attachment 251796 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 6 Build Bot 2015-04-27 18:09:34 PDT
Comment on attachment 251786 [details]
Patch for EWS

Attachment 251786 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4656464618586112

New failing tests:
fast/dom/null-document-location-assign-crash.html
http/tests/security/xss-DENIED-xsl-document-securityOrigin.xml
http/tests/security/xss-DENIED-synchronous-frame-load-in-javascript-url.html
fast/dom/null-document-location-href-put-crash.html
fast/dom/null-document-location-put-crash.html
fast/dom/null-document-location-replace-crash.html
http/tests/security/postMessage/javascript-page-still-sends-origin.html
fast/dom/null-document-window-open-crash.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-window-open.html
Comment 7 Build Bot 2015-04-27 18:09:38 PDT
Created attachment 251800 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 8 Brady Eidson 2015-04-28 09:42:28 PDT
Created attachment 251852 [details]
Patch EWS
Comment 9 Brady Eidson 2015-04-28 10:38:29 PDT
Created attachment 251858 [details]
Patch for EWS
Comment 10 Brady Eidson 2015-04-28 11:30:32 PDT
Created attachment 251869 [details]
Patch v1
Comment 11 Alexey Proskuryakov 2015-04-28 11:41:34 PDT
Comment on attachment 251869 [details]
Patch v1

Is there any word that can be used instead of "request"? It's unpleasant to see code that deals with both ResourceRequest and FrameLoadRequest.
Comment 12 Brady Eidson 2015-04-28 11:49:33 PDT
(In reply to comment #11)
> Comment on attachment 251869 [details]
> Patch v1
> 
> Is there any word that can be used instead of "request"? It's unpleasant to
> see code that deals with both ResourceRequest and FrameLoadRequest.

I agree completely.

If we come up with a good alternative name, I would be happy to go do the rename in a different "rename-only" patch.
Comment 13 WebKit Commit Bot 2015-04-28 12:39:28 PDT
Comment on attachment 251869 [details]
Patch v1

Clearing flags on attachment: 251869

Committed r183498: <http://trac.webkit.org/changeset/183498>
Comment 14 WebKit Commit Bot 2015-04-28 12:39:31 PDT
All reviewed patches have been landed.  Closing bug.