[subresource_filter] Implement the "Smart" UI on Android

chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h

Index: chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h
diff --git a/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h b/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h
index 530e4362e146403031a945e98caa2ca443281ede..2614d4421aaef6e4e0c978103d18df910e77e853 100644
--- a/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h
+++ b/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h
@@ -5,19 +5,30 @@
 #ifndef CHROME_BROWSER_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_CONTENT_SETTINGS_MANAGER_H_
 #define CHROME_BROWSER_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_CONTENT_SETTINGS_MANAGER_H_
 
+#include <memory>
 #include <string>
+#include <utility>
 
 #include "base/macros.h"
+#include "base/time/clock.h"
+#include "base/time/time.h"
 #include "components/content_settings/core/browser/content_settings_observer.h"
+#include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "components/keyed_service/core/keyed_service.h"
 
 class ContentSettingsPattern;
+class GURL;
 class HostContentSettingsMap;
 class Profile;
 
-// This class observes subresource filter content settings changes for metrics
-// collection.
+namespace base {
+class DictionaryValue;
+}  // namespace base
+
+// This class contains helpers to get/set content and website settings related
+// to subresource filtering. It also observes content setting changes for
+// metrics collection.
 class SubresourceFilterContentSettingsManager
     : public KeyedService,
       public content_settings::Observer {

[msramek, 2017/04/13 16:08:27]

Also HistoryServiceObserver and please delete the [url.origin(), timestamp]
pairs on history url deletions.

[Charlie Harrison, 2017/04/13 17:54:01]

Done + some tests.

@@ -25,6 +36,23 @@ class SubresourceFilterContentSettingsManager
   explicit SubresourceFilterContentSettingsManager(Profile* profile);
   ~SubresourceFilterContentSettingsManager() override;
 
+  ContentSetting GetContentSetting(const GURL& url) const;
+  void SetContentSetting(const GURL& url, ContentSetting setting, bool from_ui);
+  void ClearContentSetting(const GURL& url);
+
+  // Specific logic for more intelligent UI.
+  void OnDidShowUI(const GURL& url);
+  bool ShouldShowUIForSite(const GURL& url) const;
+  bool should_use_smart_ui() const { return should_use_smart_ui_; }
+
+  void set_clock_for_testing(std::unique_ptr<base::Clock> tick_clock) {
+    clock_ = std::move(tick_clock);
+  }
+
+  // Time before showing the UI again on a domain.
+  // TODO(csharrison): Consider setting this via a finch param.
+  static const base::TimeDelta kDelayBeforeShowingInfobarAgain;

[jkarlin, 2017/04/13 12:57:18]

static duration variables must be POD

[engedy, 2017/04/13 13:23:32]

The fashionable thing to do nowadays is to make this a constexpr and initialize
it at declaration time here (while retaining a definition without an initializer
clause in the .cc file).

TimeDelta is a literal type, making this qualify for constant initialization,
and while the compiler is not required to precompute the binary representation
of the object at compile-time, apparently this happens with all our toolchains.

[jkarlin, 2017/04/13 14:02:33]

Good point, and
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/static...
agrees with you.

This is fine with me as a constexpr.

+
  private:
   // KeyedService:
   void Shutdown() override;
@@ -35,8 +63,23 @@ class SubresourceFilterContentSettingsManager
                                ContentSettingsType content_type,
                                std::string resource_identifier) override;
 
+  std::unique_ptr<base::DictionaryValue> GetWebsiteSetting(
+      const GURL& url) const;
+  void SetWebsiteSetting(const GURL& url,
+                         std::unique_ptr<base::DictionaryValue> dict);
+
   HostContentSettingsMap* settings_map_;
 
+  // A clock is injected into this class so tests can set arbitrary timestamps
+  // in website settings.
+  std::unique_ptr<base::Clock> clock_;
+
+  // Used internally so the class ignores changes to the settings that are not
+  // user initiated through the settings UI.
+  bool ignore_settings_changes_ = false;
+
+  const bool should_use_smart_ui_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterContentSettingsManager);
 };