Commit c211d8be authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot

Add a method to check Script settings without notifying client

Add a method in FrameFetchContext that can be used to check if
JavaScript is allowed. This method does not notify the
ContentSettingsClient when the JavaScript is blocked.
The method is used by Client hints to check if JavaScript
is allowed.

Bug: 822553
Change-Id: I775af88099c3973f85e3c7306c1debd27282a9af
Reviewed-on: https://chromium-review.googlesource.com/967565
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544109}
parent 127b2129
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h" #include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/content_settings/tab_specific_content_settings.h"
#include "chrome/browser/metrics/subprocess_metrics_provider.h" #include "chrome/browser/metrics/subprocess_metrics_provider.h"
#include "chrome/browser/net/url_request_mock_util.h" #include "chrome/browser/net/url_request_mock_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -170,6 +171,18 @@ class ClientHintsBrowserTest : public InProcessBrowserTest { ...@@ -170,6 +171,18 @@ class ClientHintsBrowserTest : public InProcessBrowserTest {
expect_client_hints_on_subresources_ = expect_client_hints; expect_client_hints_on_subresources_ = expect_client_hints;
} }
// Verify that the user is not notified that cookies or JavaScript were
// blocked on the webpage due to the checks done by client hints.
void VerifyContentSettingsNotNotified() const {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
EXPECT_FALSE(TabSpecificContentSettings::FromWebContents(web_contents)
->IsContentBlocked(CONTENT_SETTINGS_TYPE_COOKIES));
EXPECT_FALSE(TabSpecificContentSettings::FromWebContents(web_contents)
->IsContentBlocked(CONTENT_SETTINGS_TYPE_JAVASCRIPT));
}
const GURL& accept_ch_with_lifetime_http_local_url() const { const GURL& accept_ch_with_lifetime_http_local_url() const {
return accept_ch_with_lifetime_http_local_url_; return accept_ch_with_lifetime_http_local_url_;
} }
...@@ -594,6 +607,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest, ...@@ -594,6 +607,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest,
->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_CLIENT_HINTS, std::string(), ->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_CLIENT_HINTS, std::string(),
&host_settings); &host_settings);
EXPECT_EQ(0u, host_settings.size()); EXPECT_EQ(0u, host_settings.size());
VerifyContentSettingsNotNotified();
// Allow cookies. // Allow cookies.
cookie_settings_->SetCookieSetting(accept_ch_without_lifetime_url(), cookie_settings_->SetCookieSetting(accept_ch_without_lifetime_url(),
...@@ -648,6 +662,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest, ...@@ -648,6 +662,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest,
ui_test_utils::NavigateToURL(browser(), ui_test_utils::NavigateToURL(browser(),
without_accept_ch_without_lifetime_url()); without_accept_ch_without_lifetime_url());
EXPECT_EQ(0u, count_client_hints_headers_seen()); EXPECT_EQ(0u, count_client_hints_headers_seen());
VerifyContentSettingsNotNotified();
// Allow the cookies: Client hints should now be attached. // Allow the cookies: Client hints should now be attached.
HostContentSettingsMapFactory::GetForProfile(browser()->profile()) HostContentSettingsMapFactory::GetForProfile(browser()->profile())
...@@ -694,6 +709,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest, ...@@ -694,6 +709,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest,
->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_CLIENT_HINTS, std::string(), ->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_CLIENT_HINTS, std::string(),
&host_settings); &host_settings);
EXPECT_EQ(0u, host_settings.size()); EXPECT_EQ(0u, host_settings.size());
VerifyContentSettingsNotNotified();
// Allow the JavaScript: Client hint preferences should be persisted. // Allow the JavaScript: Client hint preferences should be persisted.
HostContentSettingsMapFactory::GetForProfile(browser()->profile()) HostContentSettingsMapFactory::GetForProfile(browser()->profile())
...@@ -748,6 +764,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest, ...@@ -748,6 +764,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest,
ui_test_utils::NavigateToURL(browser(), ui_test_utils::NavigateToURL(browser(),
without_accept_ch_without_lifetime_url()); without_accept_ch_without_lifetime_url());
EXPECT_EQ(0u, count_client_hints_headers_seen()); EXPECT_EQ(0u, count_client_hints_headers_seen());
VerifyContentSettingsNotNotified();
// Allow the Javascript: Client hints should now be attached. // Allow the Javascript: Client hints should now be attached.
HostContentSettingsMapFactory::GetForProfile(browser()->profile()) HostContentSettingsMapFactory::GetForProfile(browser()->profile())
...@@ -813,6 +830,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest, ...@@ -813,6 +830,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest,
EXPECT_EQ(3u, count_client_hints_headers_seen()); EXPECT_EQ(3u, count_client_hints_headers_seen());
EXPECT_EQ(2u, third_party_request_count_seen()); EXPECT_EQ(2u, third_party_request_count_seen());
EXPECT_EQ(0u, third_party_client_hints_count_seen()); EXPECT_EQ(0u, third_party_client_hints_count_seen());
VerifyContentSettingsNotNotified();
// Clear settings. // Clear settings.
HostContentSettingsMapFactory::GetForProfile(browser()->profile()) HostContentSettingsMapFactory::GetForProfile(browser()->profile())
...@@ -859,6 +877,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest, ...@@ -859,6 +877,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTest,
// Client hints are not attached to third party subresources even though // Client hints are not attached to third party subresources even though
// cookies are allowed only for the first party origin. // cookies are allowed only for the first party origin.
EXPECT_EQ(0u, third_party_client_hints_count_seen()); EXPECT_EQ(0u, third_party_client_hints_count_seen());
VerifyContentSettingsNotNotified();
// Allow cookies. // Allow cookies.
cookie_settings_->SetCookieSetting(accept_ch_without_lifetime_img_localhost(), cookie_settings_->SetCookieSetting(accept_ch_without_lifetime_img_localhost(),
......
...@@ -872,7 +872,7 @@ void FrameFetchContext::AddClientHintsIfNecessary( ...@@ -872,7 +872,7 @@ void FrameFetchContext::AddClientHintsIfNecessary(
// Check if |url| is allowed to run JavaScript. If not, client hints are not // Check if |url| is allowed to run JavaScript. If not, client hints are not
// attached to the requests that initiate on the render side. // attached to the requests that initiate on the render side.
if (!AllowScriptFromSource(request.Url())) { if (!AllowScriptFromSourceWithoutNotifying(request.Url())) {
return; return;
} }
...@@ -980,11 +980,20 @@ MHTMLArchive* FrameFetchContext::Archive() const { ...@@ -980,11 +980,20 @@ MHTMLArchive* FrameFetchContext::Archive() const {
} }
bool FrameFetchContext::AllowScriptFromSource(const KURL& url) const { bool FrameFetchContext::AllowScriptFromSource(const KURL& url) const {
if (AllowScriptFromSourceWithoutNotifying(url))
return true;
ContentSettingsClient* settings_client = GetContentSettingsClient();
if (settings_client)
settings_client->DidNotAllowScript();
return false;
}
bool FrameFetchContext::AllowScriptFromSourceWithoutNotifying(
const KURL& url) const {
ContentSettingsClient* settings_client = GetContentSettingsClient(); ContentSettingsClient* settings_client = GetContentSettingsClient();
Settings* settings = GetSettings(); Settings* settings = GetSettings();
if (settings_client && !settings_client->AllowScriptFromSource( if (settings_client && !settings_client->AllowScriptFromSource(
!settings || settings->GetScriptEnabled(), url)) { !settings || settings->GetScriptEnabled(), url)) {
settings_client->DidNotAllowScript();
return false; return false;
} }
return true; return true;
...@@ -1233,7 +1242,7 @@ void FrameFetchContext::ParseAndPersistClientHints( ...@@ -1233,7 +1242,7 @@ void FrameFetchContext::ParseAndPersistClientHints(
if (persist_duration.InSeconds() <= 0) if (persist_duration.InSeconds() <= 0)
return; return;
if (!AllowScriptFromSource(response.Url())) { if (!AllowScriptFromSourceWithoutNotifying(response.Url())) {
// Do not persist client hint preferences if the JavaScript is disabled. // Do not persist client hint preferences if the JavaScript is disabled.
return; return;
} }
......
...@@ -240,6 +240,14 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext { ...@@ -240,6 +240,14 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext {
void ParseAndPersistClientHints(const ResourceResponse&); void ParseAndPersistClientHints(const ResourceResponse&);
void SetFirstPartyCookieAndRequestorOrigin(ResourceRequest&); void SetFirstPartyCookieAndRequestorOrigin(ResourceRequest&);
// Returns true if execution of scripts from the url are allowed. Compared to
// AllowScriptFromSource(), this method does not generate any
// notification to the |ContentSettingsClient| that the execution of the
// script was blocked. This method should be called only when there is a need
// to check the settings, and where blocked setting doesn't really imply that
// JavaScript was blocked from being executed.
bool AllowScriptFromSourceWithoutNotifying(const KURL&) const;
Member<DocumentLoader> document_loader_; Member<DocumentLoader> document_loader_;
Member<Document> document_; Member<Document> document_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment