[webkit-reviews] review granted: [Bug 209368] Add checks for app-bound navigations when evaluating user style sheets : [Attachment 394266] Patch

Mon Mar 23 10:40:27 PDT 2020

Brent Fulgham <bfulgham at webkit.org> has granted katherine_cheney at apple.com's
request for review:
Bug 209368: Add checks for app-bound navigations when evaluating user style
sheets
https://bugs.webkit.org/show_bug.cgi?id=209368

Attachment 394266: Patch

https://bugs.webkit.org/attachment.cgi?id=394266&action=review

--- Comment #5 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 394266
  --> https://bugs.webkit.org/attachment.cgi?id=394266
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=394266&action=review

Awesome! Thank you for creating useful tests for this. r=me, but please add the
logging I suggest.

> Source/WebCore/page/Page.cpp:3078
> +    if (m_mainFrame->loader().client().hasNavigatedAwayFromAppBoundDomain())

I feel like we should issue some kind of console message so developers will
know what's going on. Look for instances of 'document->addConsoleMessage' (or
context->addConsoleMessage) for examples.

> Source/WebCore/style/StyleScopeRuleSets.cpp:98
> +	  
collectRulesFromUserStyleSheets(extensionStyleSheets.injectedUserStyleSheets(),
tempUserStyle.get(), mediaQueryEvaluator);

I suggest:

auto* page = m_styleResolver.document().page();
if (page &&
page->mainFrame().loader().client().hasNavigatedAwayFromAppBoundDomain())
    m_styleResolver.document().addConsoleMessage(MessageSource::Security,
MessageLevel::Warning, "Ignoring user style sheet for non-app bound
domain."_s);
else

collectRulesFromUserStyleSheets(extensionStyleSheets.injectedUserStyleSheets(),
tempUserStyle.get(), mediaQueryEvaluator);

> Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:76
> +	   response = @"<body style='background-color: red;'><iframe
src='in-app-browser:///in-app-browser-privacy-test-user-style-sheets'></iframe>
</body>";

Nice!