Add WKNavigation/WKNavigationAction related SPI This includes: - Exposing the WKNavigation on WKNavigationAction if there is one - Exposing a C-API method to start a load and get back a WKNavigation
<rdar://problem/42387710>
Created attachment 345394 [details] Patch
Comment on attachment 345394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345394&action=review > Tools/ChangeLog:13 > +2018-07-19 Brady Eidson <beidson@apple.com> What's going on here
Created attachment 345395 [details] Patch
Comment on attachment 345395 [details] Patch r=me
Comment on attachment 345395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345395&action=review > Source/WebKit/UIProcess/API/APINavigationAction.h:82 > + NavigationAction(WebKit::NavigationActionData&& navigationActionData, API::FrameInfo* sourceFrame, API::FrameInfo* targetFrame, std::optional<WTF::String> targetFrameName, WebCore::ResourceRequest&& request, const WebCore::URL& originalURL, bool shouldOpenAppLinks, RefPtr<UserInitiatedAction>&& userInitiatedAction) Can't you use a default value instead of making another constructor? API::Navigation* mainFrameNavigation = nullptr
Comment on attachment 345395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345395&action=review > Source/WebKit/UIProcess/API/Cocoa/WKNavigationActionPrivate.h:59 > +@property (nonatomic, readonly) WKNavigation *_mainFrameNavigation WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); This feels messy. Is there really no way to access this from the client? We already tell the client about each navigation, and the client can know whether the frame is the main frame or not. > Source/WebKit/UIProcess/WebPageProxy.cpp:4026 > + API::Navigation *mainFrameNavigation = frame->isMainFrame() ? navigation.get() : nullptr; * is in the wrong place.
(In reply to Alex Christensen from comment #7) > Comment on attachment 345395 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=345395&action=review > > > Source/WebKit/UIProcess/API/Cocoa/WKNavigationActionPrivate.h:59 > > +@property (nonatomic, readonly) WKNavigation *_mainFrameNavigation WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); > > This feels messy. Is there really no way to access this from the client? > We already tell the client about each navigation, and the client can know > whether the frame is the main frame or not. We do not tell the client about every navigation, that's actually wrong. Regardless, at "decide policy" time the client has no way of knowing which navigation is being considered, and that ability is what this patch adds (Note: Only main frames have WKNavigations, hence the name of the variable) > > Source/WebKit/UIProcess/WebPageProxy.cpp:4026 > > + API::Navigation *mainFrameNavigation = frame->isMainFrame() ? navigation.get() : nullptr; > > * is in the wrong place. Indeed will fix.
(In reply to Alex Christensen from comment #6) > Comment on attachment 345395 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=345395&action=review > > > Source/WebKit/UIProcess/API/APINavigationAction.h:82 > > + NavigationAction(WebKit::NavigationActionData&& navigationActionData, API::FrameInfo* sourceFrame, API::FrameInfo* targetFrame, std::optional<WTF::String> targetFrameName, WebCore::ResourceRequest&& request, const WebCore::URL& originalURL, bool shouldOpenAppLinks, RefPtr<UserInitiatedAction>&& userInitiatedAction) > > Can't you use a default value instead of making another constructor? > API::Navigation* mainFrameNavigation = nullptr Will do.
(In reply to Brady Eidson from comment #9) > (In reply to Alex Christensen from comment #6) > > Comment on attachment 345395 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=345395&action=review > > > > > Source/WebKit/UIProcess/API/APINavigationAction.h:82 > > > + NavigationAction(WebKit::NavigationActionData&& navigationActionData, API::FrameInfo* sourceFrame, API::FrameInfo* targetFrame, std::optional<WTF::String> targetFrameName, WebCore::ResourceRequest&& request, const WebCore::URL& originalURL, bool shouldOpenAppLinks, RefPtr<UserInitiatedAction>&& userInitiatedAction) > > > > Can't you use a default value instead of making another constructor? > > API::Navigation* mainFrameNavigation = nullptr > > Will do. Actually, no, sorry I misunderstood in the moment. I think default values to arguments to function calls are bad. :P
Created attachment 345416 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 345416 [details] Patch Attachment 345416 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8595681 New failing tests: http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html
Created attachment 345428 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Layout test failure is unrelated.
Bizarre, it claims 32bit mac build broke but the output doesn't say why - the compilation of the file it claims failed isn't actually in the output. Will build 32-bit locally I guess... =/
(In reply to Brady Eidson from comment #16) > Bizarre, it claims 32bit mac build broke but the output doesn't say why - > the compilation of the file it claims failed isn't actually in the output. > > Will build 32-bit locally I guess... =/ I can't even get passed JSC locally with 32 bit. resubmitting the patch to see if i get more helpful info
Created attachment 345442 [details] Patch
Created attachment 345448 [details] Patch
Comment on attachment 345448 [details] Patch Clearing flags on attachment: 345448 Committed r234052: <https://trac.webkit.org/changeset/234052>
All reviewed patches have been landed. Closing bug.
<rdar://problem/42435157>