Add SPI for sending WebsiteSettings to WebProcess during navigation
Created attachment 296383 [details] Patch
Created attachment 296385 [details] Patch
Created attachment 296396 [details] Patch
Comment on attachment 296396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296396&action=review > Source/WebCore/loader/DocumentLoader.h:235 > + std::optional<bool> userContentExtensionsDisabled() const { return m_userContentExtensionsDisabled; } > + void setUserContentExtensionsDisabled(std::optional<bool> disabled) { m_userContentExtensionsDisabled = disabled; } Using std::optional<bool> is pretty confusing, it seems like you should be able to just have it be a boolean (and using enabled by default seems easier to understand too). > Source/WebKit2/UIProcess/API/Cocoa/_WKWebsitePolicies.h:33 > +@property (nonatomic) BOOL contentBlockersDisabled; Again, I think this should be contentBlockersEnabled.
Created attachment 296449 [details] Patch
Created attachment 296470 [details] Patch
Comment on attachment 296470 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296470&action=review > Source/WebKit2/UIProcess/API/APIWebsitePolicies.h:37 > + explicit WebsitePolicies() = default; > + virtual ~WebsitePolicies() = default; I'd put these out of line so we won't end up emitting a vtable wherever we use RefPtr<WebsitePolicies>. > Source/WebKit2/UIProcess/API/APIWebsitePolicies.h:45 > + Extra newline here. > Source/WebKit2/UIProcess/API/Cocoa/WKNavigationDelegatePrivate.h:68 > +- (void)_webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy, _WKWebsitePolicies *))decisionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); It's a little ugly that we now have an extra parameter here that only makes sense for calling it with WKNavigationActionPolicyAllow. I think if we want to make this API, we should use an object instead, similar to WebPolicyListener (or whatever it's called).. > Source/WebKit2/UIProcess/API/Cocoa/_WKWebsitePoliciesInternal.h:44 > + @package @package shouldn't be indented. > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:819 > + // Only setUserContentExtensionsEnabled if it hasn't already been disabled by reloading without content blockers. > + if (documentLoader->userContentExtensionsEnabled()) > + documentLoader->setUserContentExtensionsEnabled(websitePolicies.contentBlockersEnabled); Does that mean that it's impossible to implement "reload _with_ content blockers"?
Comment on attachment 296470 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296470&action=review >> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:819 >> + documentLoader->setUserContentExtensionsEnabled(websitePolicies.contentBlockersEnabled); > > Does that mean that it's impossible to implement "reload _with_ content blockers"? yes. we would definitely need optional bools for that.
Comment on attachment 296470 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296470&action=review >> Source/WebKit2/UIProcess/API/Cocoa/WKNavigationDelegatePrivate.h:68 >> +- (void)_webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy, _WKWebsitePolicies *))decisionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); > > It's a little ugly that we now have an extra parameter here that only makes sense for calling it with WKNavigationActionPolicyAllow. > > I think if we want to make this API, we should use an object instead, similar to WebPolicyListener (or whatever it's called).. Ok, I'll land this as-is. If we make it API, we would need to rename it anyways. The _ is the only difference between this and an existing selector's name.
http://trac.webkit.org/changeset/209558