Skip to content

Commit

Permalink
[subresource_filter] Implement the "Smart" UI on Android
Browse files Browse the repository at this point in the history 
This patch implements a V1 for the "smart" UI, which just logs a
timestamp every time the UI is shown, and will not show the UI on
subsequent navigations if it is within 30 minutes of a previous impression.

This is implemented using a new WebsiteSetting, which holds a timestamp.

BUG=689992

Review-Url: https://codereview.chromium.org/2795053002
Cr-Commit-Position: refs/heads/master@{#470217}
  • Loading branch information
@csharrison
csharrison authored and Commit bot committed May 9, 2017
1 parent 39c4cda commit 3f8cdfd
Show file tree
Hide file tree
Showing 15 changed files with 594 additions and 58 deletions.
61 changes: 25 additions & 36 deletions chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h"
#include "chrome/browser/browser_process.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/infobars/infobar_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h"
#include "chrome/browser/subresource_filter/subresource_filter_profile_context.h"
#include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
#include "chrome/browser/ui/android/content_settings/subresource_filter_infobar_delegate.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
Expand All @@ -25,7 +26,9 @@
#include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/common/activation_level.h"
#include "components/subresource_filter/core/common/activation_scope.h"
#include "components/subresource_filter/core/common/activation_state.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"

Expand All @@ -35,9 +38,10 @@ ChromeSubresourceFilterClient::ChromeSubresourceFilterClient(
content::WebContents* web_contents)
: web_contents_(web_contents), did_show_ui_for_navigation_(false) {
DCHECK(web_contents);
SubresourceFilterProfileContextFactory::EnsureForProfile(
Profile::FromBrowserContext(web_contents_->GetBrowserContext()));

SubresourceFilterProfileContext* context =
SubresourceFilterProfileContextFactory::GetForProfile(
Profile::FromBrowserContext(web_contents_->GetBrowserContext()));
settings_manager_ = context->settings_manager();
subresource_filter::ContentSubresourceFilterDriverFactory::
CreateForWebContents(web_contents, this);
}
Expand Down Expand Up @@ -85,21 +89,28 @@ void ChromeSubresourceFilterClient::ToggleNotificationVisibility(
bool visibility) {
if (did_show_ui_for_navigation_ && visibility)
return;

did_show_ui_for_navigation_ = visibility;
TabSpecificContentSettings* content_settings =
TabSpecificContentSettings::FromWebContents(web_contents_);
did_show_ui_for_navigation_ = false;

// |visibility| is false when a new navigation starts.
if (visibility) {
content_settings->OnContentBlocked(
CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER);
LogAction(kActionUIShown);
const GURL& top_level_url = web_contents_->GetLastCommittedURL();
if (!settings_manager_->ShouldShowUIForSite(top_level_url)) {
LogAction(kActionUISuppressed);
return;
}
#if defined(OS_ANDROID)
InfoBarService* infobar_service =
InfoBarService::FromWebContents(web_contents_);
SubresourceFilterInfobarDelegate::Create(infobar_service);
#endif
TabSpecificContentSettings* content_settings =
TabSpecificContentSettings::FromWebContents(web_contents_);
content_settings->OnContentBlocked(
CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER);

LogAction(kActionUIShown);
did_show_ui_for_navigation_ = true;
settings_manager_->OnDidShowUI(top_level_url);
} else {
LogAction(kActionNavigationStarted);
}
Expand All @@ -110,22 +121,12 @@ bool ChromeSubresourceFilterClient::ShouldSuppressActivation(
const GURL& url(navigation_handle->GetURL());
return navigation_handle->IsInMainFrame() &&
(whitelisted_hosts_.find(url.host()) != whitelisted_hosts_.end() ||
GetContentSettingForUrl(url) == CONTENT_SETTING_BLOCK);
settings_manager_->GetSitePermission(url) == CONTENT_SETTING_BLOCK);
}

void ChromeSubresourceFilterClient::WhitelistByContentSettings(
const GURL& url) {
// Whitelist via content settings.
Profile* profile =
Profile::FromBrowserContext(web_contents_->GetBrowserContext());
DCHECK(profile);
HostContentSettingsMap* settings_map =
HostContentSettingsMapFactory::GetForProfile(profile);
settings_map->SetContentSettingDefaultScope(
url, url, ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER,
std::string(), CONTENT_SETTING_BLOCK);

LogAction(kActionContentSettingsBlockedFromUI);
const GURL& top_level_url) {
settings_manager_->WhitelistSite(top_level_url);
}

void ChromeSubresourceFilterClient::WhitelistInCurrentWebContents(
Expand All @@ -140,18 +141,6 @@ void ChromeSubresourceFilterClient::LogAction(SubresourceFilterAction action) {
kActionLastEntry);
}

ContentSetting ChromeSubresourceFilterClient::GetContentSettingForUrl(
const GURL& url) {
Profile* profile =
Profile::FromBrowserContext(web_contents_->GetBrowserContext());
DCHECK(profile);
HostContentSettingsMap* settings_map =
HostContentSettingsMapFactory::GetForProfile(profile);
return settings_map->GetContentSetting(
url, url, ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER,
std::string());
}

subresource_filter::VerifiedRulesetDealer::Handle*
ChromeSubresourceFilterClient::GetRulesetDealer() {
subresource_filter::ContentRulesetService* ruleset_service =
Expand Down
19 changes: 17 additions & 2 deletions chrome/browser/subresource_filter/chrome_subresource_filter_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "content/public/browser/web_contents_user_data.h"

class GURL;
class SubresourceFilterContentSettingsManager;

namespace content {
class NavigationHandle;
Expand Down Expand Up @@ -45,7 +46,8 @@ enum SubresourceFilterAction {
// Content setting updated automatically via the standard UI.
kActionContentSettingsBlockedFromUI,

// Content settings which target specific origins (e.g. no wildcards).
// Content settings which target specific origins (e.g. no wildcards). These
// updates do not include updates from the main UI.
kActionContentSettingsAllowed,
kActionContentSettingsBlocked,

Expand All @@ -61,6 +63,16 @@ enum SubresourceFilterAction {
// flexible.
kActionContentSettingsWildcardUpdate,

// The UI was suppressed due to "smart" logic which tries not to spam the UI
// on navigations on the same origin within a certain time.
kActionUISuppressed,

// The feature was blocked via content setting manually while smart UI was
// suppressing the UI. Potentially indicates that the smart UI is too
// aggressive if this happens frequently. This is a reported alongside
// kActionContentSettingsBlocked if the UI is currently in suppressed mode.
kActionContentSettingsBlockedWhileUISuppressed,

kActionLastEntry
};

Expand Down Expand Up @@ -94,8 +106,11 @@ class ChromeSubresourceFilterClient
static void LogAction(SubresourceFilterAction action);

private:
ContentSetting GetContentSettingForUrl(const GURL& url);
std::set<std::string> whitelisted_hosts_;

// Owned by the profile.
SubresourceFilterContentSettingsManager* settings_manager_;

content::WebContents* web_contents_;
bool did_show_ui_for_navigation_;

Expand Down
90 changes: 88 additions & 2 deletions chrome/browser/subresource_filter/subresource_filter_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/test/histogram_tester.h"
#include "base/test/simple_test_clock.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/content_settings/tab_specific_content_settings.h"
Expand All @@ -26,6 +27,9 @@
#include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
#include "chrome/browser/safe_browsing/v4_test_utils.h"
#include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
#include "chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h"
#include "chrome/browser/subresource_filter/subresource_filter_profile_context.h"
#include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
#include "chrome/browser/subresource_filter/test_ruleset_publisher.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
Expand Down Expand Up @@ -262,6 +266,13 @@ class SubresourceFilterBrowserTest : public InProcessBrowserTest {
content::SetupCrossSiteRedirector(embedded_test_server());
ASSERT_TRUE(embedded_test_server()->Start());
ResetConfigurationToEnableOnPhishingSites();

settings_manager_ = SubresourceFilterProfileContextFactory::GetForProfile(
browser()->profile())
->settings_manager();
#if defined(OS_ANDROID)
EXPECT_TRUE(settings_manager->should_use_smart_ui());
#endif
}

GURL GetTestUrl(const std::string& relative_url) {
Expand Down Expand Up @@ -292,6 +303,10 @@ class SubresourceFilterBrowserTest : public InProcessBrowserTest {
return browser()->tab_strip_model()->GetActiveWebContents();
}

SubresourceFilterContentSettingsManager* settings_manager() {
return settings_manager_;
}

content::RenderFrameHost* FindFrameByName(const std::string& name) {
return content::FrameMatchingPredicate(
web_contents(), base::Bind(&content::FrameMatchesName, name));
Expand Down Expand Up @@ -383,6 +398,9 @@ class SubresourceFilterBrowserTest : public InProcessBrowserTest {
// Owned by the V4GetHashProtocolManager.
safe_browsing::TestV4GetHashProtocolManagerFactory* v4_get_hash_factory_;

// Owned by the profile.
SubresourceFilterContentSettingsManager* settings_manager_;

DISALLOW_COPY_AND_ASSIGN(SubresourceFilterBrowserTest);
};

Expand Down Expand Up @@ -825,6 +843,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
ASSERT_NO_FATAL_FAILURE(
SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
GURL url(GetTestUrl(kTestFrameSetPath));
GURL a_url(embedded_test_server()->GetURL(
"a.com", "/subresource_filter/frame_with_included_script.html"));
ConfigureAsPhishingURL(url);
base::HistogramTester tester;
ui_test_utils::NavigateToURL(browser(), url);
Expand All @@ -834,8 +854,10 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
EXPECT_FALSE(IsDynamicScriptElementLoaded(FindFrameByName("five")));
tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown,
1);
// Check that bubble is shown for new navigation.
ui_test_utils::NavigateToURL(browser(), url);
// Check that bubble is shown for new navigation. Must be cross site to avoid
// triggering smart UI on Android.
ConfigureAsPhishingURL(a_url);
ui_test_utils::NavigateToURL(browser(), a_url);
tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown,
2);
}
Expand Down Expand Up @@ -1004,6 +1026,70 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
}

// Test the "smart" UI, aka the logic to hide the UI on subsequent same-domain
// navigations, until a certain time threshold has been reached. This is an
// android-only feature.
IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
DoNotShowUIUntilThresholdReached) {
ASSERT_NO_FATAL_FAILURE(
SetRulesetToDisallowURLsWithPathSuffix("included_script.js"));
GURL a_url(embedded_test_server()->GetURL(
"a.com", "/subresource_filter/frame_with_included_script.html"));
GURL b_url(embedded_test_server()->GetURL(
"b.com", "/subresource_filter/frame_with_included_script.html"));
// Test utils only support one blacklisted site at a time.
// TODO(csharrison): Add support for more than one URL.
ConfigureAsPhishingURL(a_url);

// Cast is safe because this is the only type of client in non-unittest code.
ChromeSubresourceFilterClient* client =
static_cast<ChromeSubresourceFilterClient*>(
ContentSubresourceFilterDriverFactory::FromWebContents(web_contents())
->client());
auto test_clock = base::MakeUnique<base::SimpleTestClock>();
base::SimpleTestClock* raw_clock = test_clock.get();
settings_manager()->set_clock_for_testing(std::move(test_clock));

base::HistogramTester histogram_tester;

// First load should trigger the UI.
ui_test_utils::NavigateToURL(browser(), a_url);
EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
EXPECT_TRUE(client->did_show_ui_for_navigation());

histogram_tester.ExpectBucketCount(kSubresourceFilterActionsHistogram,
kActionUISuppressed, 0);

// Second load should not trigger the UI, but should still filter content.
ui_test_utils::NavigateToURL(browser(), a_url);
EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));

bool use_smart_ui = settings_manager()->should_use_smart_ui();
EXPECT_EQ(client->did_show_ui_for_navigation(), !use_smart_ui);

histogram_tester.ExpectBucketCount(kSubresourceFilterActionsHistogram,
kActionUISuppressed, use_smart_ui ? 1 : 0);

ConfigureAsPhishingURL(b_url);

// Load to another domain should trigger the UI.
ui_test_utils::NavigateToURL(browser(), b_url);
EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
EXPECT_TRUE(client->did_show_ui_for_navigation());

ConfigureAsPhishingURL(a_url);

// Fast forward the clock, and a_url should trigger the UI again.
raw_clock->Advance(
SubresourceFilterContentSettingsManager::kDelayBeforeShowingInfobarAgain);
ui_test_utils::NavigateToURL(browser(), a_url);
EXPECT_FALSE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
EXPECT_TRUE(client->did_show_ui_for_navigation());

histogram_tester.ExpectBucketCount(kSubresourceFilterActionsHistogram,
kActionUISuppressed, use_smart_ui ? 1 : 0);
}

IN_PROC_BROWSER_TEST_P(SubresourceFilterWebSocketBrowserTest, BlockWebSocket) {
GURL url(GetTestUrl(
base::StringPrintf("subresource_filter/page_with_websocket.html?%s",
Expand Down
Loading

0 comments on commit 3f8cdfd

Please sign in to comment.