[subresource_filter] Implement the "Smart" UI on Android

chrome/browser/subresource_filter/chrome_subresource_filter_client.cc

Index: chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
diff --git a/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc b/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
index 43b8eb5776dedf75084cfbcb99045e15b49d8f39..b77cdc962795ed87d5915adda5a9bb69210a9d66 100644
--- a/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
+++ b/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
@@ -6,7 +6,10 @@
 
 #include <string>
 
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/time/default_clock.h"
+#include "base/values.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"
@@ -16,9 +19,22 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 
+namespace {
+
+// Key into the website setting dict for the smart UI.
+const char kUILastShowTime[] = "LastShowTime";
+
+// Number of hours before showing the UI again on a domain.
+// TODO(csharrison): Consider setting this via a finch param.
+const int kUIShowThresholdHours = 24;

[engedy, 2017/04/04 13:35:59]

nit: constexpr base::TimeDelta k...Threshold = base::TimeDelta::FromHours(24);

[Charlie Harrison, 2017/04/10 14:58:53]

Done.

+
+}  // namespace
+
 ChromeSubresourceFilterClient::ChromeSubresourceFilterClient(
     content::WebContents* web_contents)
-    : web_contents_(web_contents), shown_for_navigation_(false) {
+    : clock_(base::MakeUnique<base::DefaultClock>(base::DefaultClock())),
+      web_contents_(web_contents),
+      shown_for_navigation_(false) {
   DCHECK(web_contents);
   // Ensure the content settings observer is initialized.
   SubresourceFilterContentSettingsObserverFactory::EnsureForProfile(
@@ -28,24 +44,28 @@ ChromeSubresourceFilterClient::ChromeSubresourceFilterClient(
 ChromeSubresourceFilterClient::~ChromeSubresourceFilterClient() {}
 
 void ChromeSubresourceFilterClient::ToggleNotificationVisibility(
+    const GURL& url,

[engedy, 2017/04/04 13:35:59]

nit: Have you considered simply using web_contents_->GetLastCommittedURL() here?
Would that be unreliable?

[Charlie Harrison, 2017/04/10 14:58:53]

Ahh, much better :) Done.

     bool visibility) {
   if (shown_for_navigation_ && visibility)
     return;
-
-  shown_for_navigation_ = visibility;
-  TabSpecificContentSettings* content_settings =
-      TabSpecificContentSettings::FromWebContents(web_contents_);
+  shown_for_navigation_ = false;
 
   // |visibility| is false when a new navigation starts.
   if (visibility) {
-    content_settings->OnContentBlocked(
-        CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER);
-    LogAction(kActionUIShown);
+    if (!ShouldShowUIForSite(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);
+    OnUIShown(url);
   } else {
     LogAction(kActionNavigationStarted);
   }
@@ -53,18 +73,16 @@ void ChromeSubresourceFilterClient::ToggleNotificationVisibility(
 
 bool ChromeSubresourceFilterClient::IsWhitelistedByContentSettings(
     const GURL& url) {
-  return GetContentSettingForUrl(url) == CONTENT_SETTING_BLOCK;
+  ContentSetting setting = GetSettingsMap()->GetContentSetting(
+      url, url, ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER,
+      std::string());
+  return setting == 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(
+  GetSettingsMap()->SetContentSettingDefaultScope(
       url, url, ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER,
       std::string(), CONTENT_SETTING_BLOCK);
 
@@ -77,14 +95,57 @@ void ChromeSubresourceFilterClient::LogAction(SubresourceFilterAction action) {
                             kActionLastEntry);
 }
 
-ContentSetting ChromeSubresourceFilterClient::GetContentSettingForUrl(
-    const GURL& url) {
+bool ChromeSubresourceFilterClient::UsingSmartUI() const {

[engedy, 2017/04/04 13:35:59]

nit: How about calling this ShouldUseSmartUI?

[Charlie Harrison, 2017/04/10 14:58:53]

Done.

+#if defined(OS_ANDROID)
+  return true;
+#else
+  return false;
+#endif
+}
+
+HostContentSettingsMap* ChromeSubresourceFilterClient::GetSettingsMap() const {

[engedy, 2017/04/04 13:35:59]

nit: Can we make this a global function in an anonymous namespace?

[Charlie Harrison, 2017/04/10 14:58:53]

Would prefer not to since we'd need to inject the web_contents_ into every call
site.

[engedy, 2017/04/12 14:02:50]

Acknowledged.

   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());
+  DCHECK(settings_map);
+  return settings_map;
+}
+
+void ChromeSubresourceFilterClient::OnUIShown(const GURL& url) {
+  LogAction(kActionUIShown);
+  shown_for_navigation_ = true;
+
+  if (!UsingSmartUI())
+    return;
+  auto dict = base::MakeUnique<base::DictionaryValue>();
+  double now = clock_->Now().ToDoubleT();
+  dict->SetDouble(kUILastShowTime, now);
+  GetSettingsMap()->SetWebsiteSettingDefaultScope(
+      url, url,

[engedy, 2017/04/04 13:35:59]

For REQUESTING_ORIGIN_ONLY_SCOPE website settings, |secondary_url| is ignored,
so let's just pass GURL().

[Charlie Harrison, 2017/04/10 14:58:53]

Done.

+      ContentSettingsType::CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER_DATA,
+      std::string(), std::move(dict));
+}
+
+bool ChromeSubresourceFilterClient::ShouldShowUIForSite(const GURL& url) const {
+  if (!UsingSmartUI())
+    return true;
+
+  std::unique_ptr<base::DictionaryValue> dict =
+      base::DictionaryValue::From(GetSettingsMap()->GetWebsiteSetting(
+          url, url, CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER_DATA,
+          std::string(), nullptr));
+  if (!dict)
+    return true;
+
+  double last_shown_time_double = 0;
+  if (dict->GetDouble(kUILastShowTime, &last_shown_time_double)) {
+    base::Time last_shown = base::Time::FromDoubleT(last_shown_time_double);
+    if (clock_->Now() - last_shown <
+        base::TimeDelta::FromHours(kUIShowThresholdHours)) {
+      return false;
+    }
+  }
+  return true;
 }