The limitsNavigationsToAppBoundDomains flag in the WKWebView configuration should opt a WebView into AppBound domain behavior and limit navigations to the domains in the Info.plist
<rdar://problem/62065241>
Created attachment 397024 [details] Patch
I also think we can eventually clean this up and merge m_isNavigatingToAppBoundDomain and m_hasNavigatedAwayFromAppBoundDomain into one Optional parameter that defaults to WTF::nullopt. But this would be a large refactoring and is probably better for a future patch.
Comment on attachment 397024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397024&action=review Looks good. r=me, but please consider my suggestion for renaming. > Source/WebKit/UIProcess/WebPageProxy.cpp:3130 > +bool WebPageProxy::setIsNavigatingToAppBoundDomain(bool isMainFrame, const URL& requestURL, Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain) I think this would be clearer with a name change: Maybe something like: bool WebPageProxy::setIsNavigatingToAppBoundDomainAndCheckIfPermitted(...) > Source/WebKit/UIProcess/WebPageProxy.cpp:3144 > + if (*isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::Yes) { I would suggest reversing this check, and doing an early return: if (*isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::No) return false; Also: If we make the decision to return early here, should we be setting "m_hasNavigatedAwayFromAppBoundDomain = NavigatedAwayFromAppBoundDomain::No"? > Source/WebKit/UIProcess/WebPageProxy.cpp:3150 > } Maybe this could then be an 'else' block, and both fall through to the 'true' at the bottom.
(In reply to katherine_cheney from comment #3) > I also think we can eventually clean this up and merge > m_isNavigatingToAppBoundDomain and m_hasNavigatedAwayFromAppBoundDomain into > one Optional parameter that defaults to WTF::nullopt. But this would be a > large refactoring and is probably better for a future patch. Yes -- this sounds like a good next step. Might simplify the logic and make it easier to understand. :-)
Comment on attachment 397024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397024&action=review >> Source/WebKit/UIProcess/WebPageProxy.cpp:3130 >> +bool WebPageProxy::setIsNavigatingToAppBoundDomain(bool isMainFrame, const URL& requestURL, Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain) > > I think this would be clearer with a name change: Maybe something like: > > bool WebPageProxy::setIsNavigatingToAppBoundDomainAndCheckIfPermitted(...) Good idea, I'll do this >> Source/WebKit/UIProcess/WebPageProxy.cpp:3144 >> + if (*isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::Yes) { > > I would suggest reversing this check, and doing an early return: > > if (*isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::No) > return false; > > Also: If we make the decision to return early here, should we be setting "m_hasNavigatedAwayFromAppBoundDomain = NavigatedAwayFromAppBoundDomain::No"? Early return sounds good. m_hasNavigatedAwayFromAppBoundDomain defaults to No, and won't ever be changed to Yes for a webView with the m_limitsNavigationsToAppBoundDomains flag, so it should always be the case that m_hasNavigatedAwayFromAppBoundDomain = NavigatedAwayFromAppBoundDomain::No, and we shouldn't need to set it. >> Source/WebKit/UIProcess/WebPageProxy.cpp:3150 >> } > > Maybe this could then be an 'else' block, and both fall through to the 'true' at the bottom. Will do!
Created attachment 397045 [details] Patch for landing
Committed r260408: <https://trac.webkit.org/changeset/260408> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397045 [details].
Comment on attachment 397024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397024&action=review >>>> Source/WebKit/UIProcess/WebPageProxy.cpp:3144 >>>> + if (*isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::Yes) { >>> >>> I would suggest reversing this check, and doing an early return: >>> >>> if (*isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::No) >>> return false; >>> >>> Also: If we make the decision to return early here, should we be setting "m_hasNavigatedAwayFromAppBoundDomain = NavigatedAwayFromAppBoundDomain::No"? >> >> Early return sounds good. >> >> m_hasNavigatedAwayFromAppBoundDomain defaults to No, and won't ever be changed to Yes for a webView with the m_limitsNavigationsToAppBoundDomains flag, so it should always be the case that m_hasNavigatedAwayFromAppBoundDomain = NavigatedAwayFromAppBoundDomain::No, and we shouldn't need to set it. > > Early return sounds good. > > m_hasNavigatedAwayFromAppBoundDomain defaults to No, and won't ever be changed to Yes for a webView with the m_limitsNavigationsToAppBoundDomains flag, so it should always be the case that m_hasNavigatedAwayFromAppBoundDomain = NavigatedAwayFromAppBoundDomain::No, and we shouldn't need to set it. Ah -- okay. I didn't understand that.