[subresource_filter] Add metrics for UI / related things

chrome/browser/subresource_filter/subresource_filter_content_settings_observer_factory.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_CONTENT_SETTINGS_OBSERVER_FACTORY_H_
+#define CHROME_BROWSER_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_CONTENT_SETTINGS_OBSERVER_FACTORY_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "components/content_settings/core/browser/content_settings_observer.h"
+#include "components/content_settings/core/common/content_settings.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+class ContentSettingsPattern;
+class HostContentSettingsMap;
+class Profile;
+
+class SubresourceFilterContentSettingsObserver

[msramek, 2017/03/29 09:41:25]

optional nit: Putting the KeyedService to the same file as its factory seems
like a strange pattern.

I understand that we never need to directly access it, so I'm just wondering if
we need to encapsulate them together better. Or maybe put the declaration into
an anonymous namespace in the .cc file. Note that the factory declaration
doesn't need to see the observer's declaration.

[Charlie Harrison, 2017/03/29 15:23:32]

Putting this anonymous in the cc file sounds good to me.

I was originally concerned about the GetForProfile method in the factory, but
that could easily be renamed to "EnsureForProfile" or something so it doesn't
need to actually return anything.

+    : public KeyedService,
+      public content_settings::Observer {
+ public:
+  explicit SubresourceFilterContentSettingsObserver(Profile* profile);
+  ~SubresourceFilterContentSettingsObserver() override {}
+
+ private:
+  // KeyedService:
+  void Shutdown() override;
+
+  // content_settings::Observer:
+  void OnContentSettingChanged(const ContentSettingsPattern& primary_pattern,
+                               const ContentSettingsPattern& secondary_pattern,
+                               ContentSettingsType content_type,
+                               std::string resource_identifier) override;
+
+  ContentSetting GetContentSettingForUrl(const GURL& url);
+
+  HostContentSettingsMap* settings_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(SubresourceFilterContentSettingsObserver);
+};
+
+class SubresourceFilterContentSettingsObserverFactory

[msramek, 2017/03/29 09:41:25]

I share your feeling that a KeyedService seems like an overkill, but I don't
have a better idea. HCSM is scoped to a Profile, you need the observer to be
scoped to a Profile as well... Nothing else in this directory already has this
property, so this is probably the correct way to do it.

[Charlie Harrison, 2017/03/29 15:23:32]

Acknowledged.

+    : public BrowserContextKeyedServiceFactory {
+ public:
+  static SubresourceFilterContentSettingsObserver* GetForProfile(
+      Profile* profile);
+
+  static SubresourceFilterContentSettingsObserverFactory* GetInstance();
+
+  SubresourceFilterContentSettingsObserverFactory();
+
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* profile) const override;
+
+  DISALLOW_COPY_AND_ASSIGN(SubresourceFilterContentSettingsObserverFactory);
+};
+
+#endif  // CHROME_BROWSER_SUBRESOURCE_FILTER_SUBRESOURCE_FILTER_CONTENT_SETTINGS_OBSERVER_FACTORY_H_