Update content setting for app banners to store more information.

This changes the app banner content setting from being a dictionary of
app key to bool, to being a dictionary of app key to dictionary.

The sub dictionary contains one bool for whether the banner has been
blocked, and also a list of dates. The list of dates will contain at
most 14 dates, and will be used to record distinct dates on which the
banner could have been shown.

This is to allow more complex triggering. The goal is to actually show
banners if they could otherwise have been shown on a previous date in
the last 14 days.

Note the complex triggering is not currently used.

BUG=452825

Review URL: https://codereview.chromium.org/884373002

Cr-Commit-Position: refs/heads/master@{#314266}

chrome/browser/android/banners/app_banner_manager.cc

@@ -10,8 +10,8 @@
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "chrome/browser/android/banners/app_banner_metrics_ids.h"
-#include "chrome/browser/android/banners/app_banner_settings_helper.h"
 #include "chrome/browser/android/banners/app_banner_utilities.h"
+#include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_constants.h"

chrome/browser/android/banners/app_banner_settings_helper.cc

-// Copyright 2014 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.
-
-#include "chrome/browser/android/banners/app_banner_settings_helper.h"
-
-#include <algorithm>
-#include <string>
-
-#include "chrome/browser/profiles/profile.h"
-#include "components/content_settings/core/browser/host_content_settings_map.h"
-#include "components/content_settings/core/common/content_settings_pattern.h"
-#include "content/public/browser/web_contents.h"
-#include "url/gurl.h"
-
-namespace {
-std::string SanitizePackageName(std::string package_name) {
-  // DictionaryValue doesn't allow '.' in the keys.  Replace them with ' '
-  // because you can't have a package name with a ' ' in it.
-  std::replace(package_name.begin(), package_name.end(), '.', ' ');
-  return package_name;
-}
-
-// Max number of apps that a particular site may show a banner for.
-const size_t kMaxAppsPerSite = 3;
-}  // namespace
-
-bool AppBannerSettingsHelper::IsAllowed(content::WebContents* web_contents,
-                                        const GURL& origin_url,
-                                        const std::string& package_name) {
-  std::string sanitized_package_name = SanitizePackageName(package_name);
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  if (profile->IsOffTheRecord() || web_contents->GetURL() != origin_url ||
-      sanitized_package_name.empty()) {
-    return false;
-  }
-
-  // Check if this combination has been previously disabled.
-  HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
-  if (!settings)
-    return false;
-  scoped_ptr<base::Value> value =
-      settings->GetWebsiteSetting(origin_url,
-                                  origin_url,
-                                  CONTENT_SETTINGS_TYPE_APP_BANNER,
-                                  std::string(),
-                                  NULL);
-  if (!value.get()) {
-    // We've never blocked a banner on this site.
-    return true;
-  } else if (value->IsType(base::Value::TYPE_DICTIONARY)) {
-    // We expect to get a Dictionary back, where the keys are the package names.
-    base::DictionaryValue* banner_dict =
-        static_cast<base::DictionaryValue*>(value.get());
-    bool is_allowed = false;
-    if (banner_dict->GetBoolean(sanitized_package_name, &is_allowed)) {
-      return is_allowed;
-    } else {
-      return banner_dict->size() < ::kMaxAppsPerSite;
-    }
-  } else {
-    // Somehow the value we got back is not a dictionary (e.g. the settings were
-    // corrupted by malware).  Try to clear it out.
-    ContentSettingsPattern pattern(ContentSettingsPattern::FromURL(origin_url));
-    if (pattern.IsValid()) {
-      settings->SetWebsiteSetting(pattern,
-                                  ContentSettingsPattern::Wildcard(),
-                                  CONTENT_SETTINGS_TYPE_APP_BANNER,
-                                  std::string(),
-                                  NULL);
-      return true;
-    }
-  }
-
-  return false;
-}
-
-void AppBannerSettingsHelper::Block(content::WebContents* web_contents,
-                                    const GURL& origin_url,
-                                    const std::string& package_name) {
-  std::string sanitized_package_name = SanitizePackageName(package_name);
-  DCHECK(!sanitized_package_name.empty());
-
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
-  ContentSettingsPattern pattern(ContentSettingsPattern::FromURL(origin_url));
-  if (!settings || !pattern.IsValid())
-    return;
-
-  scoped_ptr<base::Value> value =
-      settings->GetWebsiteSetting(origin_url,
-                                  origin_url,
-                                  CONTENT_SETTINGS_TYPE_APP_BANNER,
-                                  std::string(),
-                                  NULL);
-  base::DictionaryValue* banner_dict = NULL;
-  if (value.get() && value->IsType(base::Value::TYPE_DICTIONARY)) {
-    banner_dict = static_cast<base::DictionaryValue*>(value.release());
-  } else {
-    banner_dict = new base::DictionaryValue();
-  }
-
-  // Update the setting and save it back.
-  banner_dict->SetBoolean(sanitized_package_name, false);
-  settings->SetWebsiteSetting(pattern,
-                              ContentSettingsPattern::Wildcard(),
-                              CONTENT_SETTINGS_TYPE_APP_BANNER,
-                              std::string(),
-                              banner_dict);
-}

chrome/browser/android/banners/app_banner_settings_helper.h

-// Copyright 2014 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_ANDROID_BANNERS_APP_BANNER_SETTINGS_HELPER_H_
-#define CHROME_BROWSER_ANDROID_BANNERS_APP_BANNER_SETTINGS_HELPER_H_
-
-#include <string>
-
-#include "base/macros.h"
-
-namespace content {
-class WebContents;
-}  // namesapce content
-
-class GURL;
-
-// Utility class for reading and updating ContentSettings for app banners.
-class AppBannerSettingsHelper {
- public:
-  // Checks if a URL is allowed to show a banner for the given package.
-  static bool IsAllowed(content::WebContents* web_contents,
-                        const GURL& origin_url,
-                        const std::string& package_name);
-
-  // Blocks a URL from showing a banner for the given package.
-  static void Block(content::WebContents* web_contents,
-                    const GURL& origin_url,
-                    const std::string& package_name);
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(AppBannerSettingsHelper);
-};
-
-#endif  // CHROME_BROWSER_ANDROID_BANNERS_APP_BANNER_SETTINGS_HELPER_H_

chrome/browser/banners/OWNERS

+benwells@chromium.org
+dfalcantara@chromium.org

chrome/browser/banners/app_banner_settings_helper.cc

+// Copyright 2014 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.
+
+#include "chrome/browser/banners/app_banner_settings_helper.h"
+
+#include <algorithm>
+#include <string>
+
+#include "chrome/browser/profiles/profile.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/content_settings/core/common/content_settings_pattern.h"
+#include "content/public/browser/web_contents.h"
+#include "net/base/escape.h"
+#include "url/gurl.h"
+
+namespace {
+
+// Max number of apps (including ServiceWorker based web apps) that a particular
+// site may show a banner for.
+const size_t kMaxAppsPerSite = 3;
+
+// Oldest could show banner event we care about, in days.
+const unsigned int kOldestCouldShowBannerEventInDays = 14;
+
+// Dictionary key to use for the 'could show banner' events.
+const char kCouldShowBannerEventsKey[] = "couldShowBannerEvents";
+
+// Dictionary key to use whether the banner has been blocked.
+const char kHasBlockedKey[] = "hasBlocked";
+
+base::Time DateFromTime(base::Time time) {
+  base::Time::Exploded exploded;
+  time.LocalExplode(&exploded);
+  exploded.hour = 0;
+  exploded.minute = 0;
+  exploded.second = 0;
+  return base::Time::FromLocalExploded(exploded);
+}
+
+scoped_ptr<base::DictionaryValue> GetOriginDict(
+    HostContentSettingsMap* settings,
+    const GURL& origin_url) {
+  if (!settings)
+    return scoped_ptr<base::DictionaryValue>();
+
+  scoped_ptr<base::Value> value = settings->GetWebsiteSetting(
+      origin_url, origin_url, CONTENT_SETTINGS_TYPE_APP_BANNER, std::string(),
+      NULL);
+  if (!value.get())
+    return make_scoped_ptr(new base::DictionaryValue());
+
+  if (!value->IsType(base::Value::TYPE_DICTIONARY))
+    return make_scoped_ptr(new base::DictionaryValue());
+
+  return make_scoped_ptr(static_cast<base::DictionaryValue*>(value.release()));
+}
+
+base::DictionaryValue* GetAppDict(base::DictionaryValue* origin_dict,
+                                  const std::string& key_name) {
+  base::DictionaryValue* app_dict = nullptr;
+  if (!origin_dict->GetDictionaryWithoutPathExpansion(key_name, &app_dict)) {
+    // Don't allow more than kMaxAppsPerSite dictionaries.
+    if (origin_dict->size() < kMaxAppsPerSite) {
+      app_dict = new base::DictionaryValue();
+      origin_dict->SetWithoutPathExpansion(key_name, make_scoped_ptr(app_dict));
+    }
+  }
+
+  return app_dict;
+}
+
+}  // namespace
+
+void AppBannerSettingsHelper::RecordCouldShowBannerEvent(
+    content::WebContents* web_contents,
+    const GURL& origin_url,
+    const std::string& package_name_or_start_url,
+    base::Time time) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  if (profile->IsOffTheRecord() || web_contents->GetURL() != origin_url ||
+      package_name_or_start_url.empty()) {
+    return;
+  }
+
+  ContentSettingsPattern pattern(ContentSettingsPattern::FromURL(origin_url));
+  if (!pattern.IsValid())
+    return;
+
+  HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
+  scoped_ptr<base::DictionaryValue> origin_dict =
+      GetOriginDict(settings, origin_url);
+  if (!origin_dict)
+    return;
+
+  base::DictionaryValue* app_dict =
+      GetAppDict(origin_dict.get(), package_name_or_start_url);
+  if (!app_dict)
+    return;
+
+  base::ListValue* could_show_list = nullptr;
+  if (!app_dict->GetList(kCouldShowBannerEventsKey, &could_show_list)) {
+    could_show_list = new base::ListValue();
+    app_dict->Set(kCouldShowBannerEventsKey, make_scoped_ptr(could_show_list));
+  }
+
+  // Trim any items that are older than we should care about. For comparisons
+  // the times are converted to local dates.
+  base::Time date = DateFromTime(time);
+  base::ValueVector::iterator it = could_show_list->begin();
+  while (it != could_show_list->end()) {
+    if ((*it)->IsType(base::Value::TYPE_DOUBLE)) {
+      double internal_date;
+      (*it)->GetAsDouble(&internal_date);
+      base::Time other_date =
+          DateFromTime(base::Time::FromInternalValue(internal_date));
+      // This date has already been added. Don't add the date again, and don't
+      // bother trimming values as it will have been done the first time the
+      // date was added (unless the local date has changed, which we can live
+      // with).
+      if (other_date == date)
+        return;
+
+      base::TimeDelta delta = date - other_date;
+      if (delta <
+          base::TimeDelta::FromDays(kOldestCouldShowBannerEventInDays)) {
+        ++it;
+        continue;
+      }
+    }
+
+    // Either this date is older than we care about, or it isn't a date, so
+    // remove it;
+    it = could_show_list->Erase(it, nullptr);
+  }
+
+  // Dates are stored in their raw form (i.e. not local dates) to be resilient
+  // to time zone changes.
+  could_show_list->AppendDouble(time.ToInternalValue());
+  settings->SetWebsiteSetting(pattern, ContentSettingsPattern::Wildcard(),
+                              CONTENT_SETTINGS_TYPE_APP_BANNER, std::string(),
+                              origin_dict.release());
+}
+
+std::vector<base::Time> AppBannerSettingsHelper::GetCouldShowBannerEvents(
+    content::WebContents* web_contents,
+    const GURL& origin_url,
+    const std::string& package_name_or_start_url) {
+  std::vector<base::Time> result;
+  if (package_name_or_start_url.empty())
+    return result;
+
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
+  scoped_ptr<base::DictionaryValue> origin_dict =
+      GetOriginDict(settings, origin_url);
+
+  if (!origin_dict)
+    return result;
+
+  base::DictionaryValue* app_dict =
+      GetAppDict(origin_dict.get(), package_name_or_start_url);
+  if (!app_dict)
+    return result;
+
+  base::ListValue* could_show_list = nullptr;
+  if (!app_dict->GetList(kCouldShowBannerEventsKey, &could_show_list))
+    return result;
+
+  for (auto value : *could_show_list) {
+    if (value->IsType(base::Value::TYPE_DOUBLE)) {
+      double internal_date;
+      value->GetAsDouble(&internal_date);
+      base::Time date = base::Time::FromInternalValue(internal_date);
+      result.push_back(date);
+    }
+  }
+
+  return result;
+}
+
+bool AppBannerSettingsHelper::IsAllowed(
+    content::WebContents* web_contents,
+    const GURL& origin_url,
+    const std::string& package_name_or_start_url) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  if (profile->IsOffTheRecord() || web_contents->GetURL() != origin_url ||
+      package_name_or_start_url.empty()) {
+    return false;
+  }
+
+  HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
+  scoped_ptr<base::DictionaryValue> origin_dict =
+      GetOriginDict(settings, origin_url);
+
+  if (!origin_dict)
+    return true;
+
+  base::DictionaryValue* app_dict =
+      GetAppDict(origin_dict.get(), package_name_or_start_url);
+  if (!app_dict)
+    return true;
+
+  bool has_blocked;
+  if (!app_dict->GetBoolean(kHasBlockedKey, &has_blocked))
+    return true;
+
+  return !has_blocked;
+}
+
+void AppBannerSettingsHelper::Block(
+    content::WebContents* web_contents,
+    const GURL& origin_url,
+    const std::string& package_name_or_start_url) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  if (profile->IsOffTheRecord() || web_contents->GetURL() != origin_url ||
+      package_name_or_start_url.empty()) {
+    return;
+  }
+
+  ContentSettingsPattern pattern(ContentSettingsPattern::FromURL(origin_url));
+  if (!pattern.IsValid())
+    return;
+
+  HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
+  scoped_ptr<base::DictionaryValue> origin_dict =
+      GetOriginDict(settings, origin_url);
+
+  if (!origin_dict)
+    return;
+
+  base::DictionaryValue* app_dict =
+      GetAppDict(origin_dict.get(), package_name_or_start_url);
+  if (!app_dict)
+    return;
+
+  // Update the setting and save it back.
+  app_dict->SetBoolean(kHasBlockedKey, true);
+  settings->SetWebsiteSetting(pattern, ContentSettingsPattern::Wildcard(),
+                              CONTENT_SETTINGS_TYPE_APP_BANNER, std::string(),
+                              origin_dict.release());
+}

chrome/browser/banners/app_banner_settings_helper.h

+// Copyright 2014 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_BANNERS_APP_BANNER_SETTINGS_HELPER_H_
+#define CHROME_BROWSER_BANNERS_APP_BANNER_SETTINGS_HELPER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/time/time.h"
+
+namespace content {
+class WebContents;
+}  // namespace content
+
+class GURL;
+
+// Utility class for reading and updating ContentSettings for app banners.
+class AppBannerSettingsHelper {
+ public:
+  // TODO(benwells): Use this method to implement smarter triggering logic.
+  // See http://crbug.com/452825.
+  // Records that a banner could have been shown for the given package or start
+  // url.
+  //
+  // These events are used to decide when banners should be shown, using a
+  // heuristic based on how many different days in a recent period of time (for
+  // example the past two weeks) the banner could have been shown. The desired
+  // effect is to have banners appear once a user has demonstrated an ongoing
+  // relationship with the app.
+  //
+  // At most one event is stored per day, and events outside the window the
+  // heuristic uses are discarded. Local times are used to enforce these rules,
+  // to ensure what we count as a day matches what the user perceives to be
+  // days.
+  static void RecordCouldShowBannerEvent(
+      content::WebContents* web_contents,
+      const GURL& origin_url,
+      const std::string& package_name_or_start_url,
+      base::Time time);
+
+  // Gets the could have been shown events that are stored for the given package
+  // or start url. This is only used for testing.
+  static std::vector<base::Time> GetCouldShowBannerEvents(
+      content::WebContents* web_contents,
+      const GURL& origin_url,
+      const std::string& package_name_or_start_url);
+
+  // Checks if a URL is allowed to show a banner for the given package or start
+  // url.
+  static bool IsAllowed(content::WebContents* web_contents,
+                        const GURL& origin_url,
+                        const std::string& package_name_or_start_url);
+
+  // Blocks a URL from showing a banner for the given package or start url.
+  static void Block(content::WebContents* web_contents,
+                    const GURL& origin_url,
+                    const std::string& package_name_or_start_url);
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(AppBannerSettingsHelper);
+};
+
+#endif  // CHROME_BROWSER_BANNERS_APP_BANNER_SETTINGS_HELPER_H_

chrome/browser/banners/app_banner_settings_helper_unittest.cc

+// Copyright 2015 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.
+
+#include <vector>
+
+#include "chrome/browser/banners/app_banner_settings_helper.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+
+namespace {
+
+const char kTestURL[] = "http://www.google.com";
+const char kTestPackageName[] = "test.package";
+
+bool IsWithinDay(base::Time time1, base::Time time2) {
+  return time1 - time2 < base::TimeDelta::FromDays(1) ||
+         time2 - time1 < base::TimeDelta::FromDays(1);
+}
+
+class AppBannerSettingsHelperTest : public ChromeRenderViewHostTestHarness {};
+
+}  // namespace
+
+TEST_F(AppBannerSettingsHelperTest, Block) {
+  GURL url(kTestURL);
+  NavigateAndCommit(url);
+
+  // Check that by default, showing the banner is allowed.
+  EXPECT_TRUE(AppBannerSettingsHelper::IsAllowed(web_contents(), url,
+                                                 kTestPackageName));
+
+  // Block the banner and test it is no longer allowed.
+  AppBannerSettingsHelper::Block(web_contents(), url, kTestPackageName);
+  EXPECT_FALSE(AppBannerSettingsHelper::IsAllowed(web_contents(), url,
+                                                  kTestPackageName));
+}
+
+TEST_F(AppBannerSettingsHelperTest, CouldShowEvents) {
+  GURL url(kTestURL);
+  NavigateAndCommit(url);
+
+  // Check that by default, there are no events recorded.
+  std::vector<base::Time> events =
+      AppBannerSettingsHelper::GetCouldShowBannerEvents(web_contents(), url,
+                                                        kTestPackageName);
+  EXPECT_TRUE(events.empty());
+
+  base::Time::Exploded exploded_reference_time;
+  exploded_reference_time.year = 2015;
+  exploded_reference_time.month = 1;
+  exploded_reference_time.day_of_month = 30;
+  exploded_reference_time.day_of_week = 5;
+  exploded_reference_time.hour = 11;
+  exploded_reference_time.minute = 0;
+  exploded_reference_time.second = 0;
+  exploded_reference_time.millisecond = 0;
+
+  base::Time reference_time =
+      base::Time::FromLocalExploded(exploded_reference_time);
+  base::Time same_day = reference_time + base::TimeDelta::FromHours(2);
+  base::Time three_days_prior = reference_time - base::TimeDelta::FromDays(3);
+  base::Time previous_fortnight =
+      reference_time - base::TimeDelta::FromDays(14);
+
+  // Test adding the first date.
+  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
+      web_contents(), url, kTestPackageName, previous_fortnight);
+
+  // It should be the only date recorded.
+  events = AppBannerSettingsHelper::GetCouldShowBannerEvents(
+      web_contents(), url, kTestPackageName);
+  EXPECT_EQ(1u, events.size());
+  EXPECT_TRUE(IsWithinDay(events[0], previous_fortnight));
+
+  // Now add the next date.
+  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
+      web_contents(), url, kTestPackageName, three_days_prior);
+
+  // Now there should be two days.
+  events = AppBannerSettingsHelper::GetCouldShowBannerEvents(
+      web_contents(), url, kTestPackageName);
+  EXPECT_EQ(2u, events.size());
+  EXPECT_TRUE(IsWithinDay(events[0], previous_fortnight));
+  EXPECT_TRUE(IsWithinDay(events[1], three_days_prior));
+
+  // Now add the reference date.
+  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
+      web_contents(), url, kTestPackageName, reference_time);
+
+  // Now there should still be two days, but the first date should have been
+  // removed.
+  events = AppBannerSettingsHelper::GetCouldShowBannerEvents(
+      web_contents(), url, kTestPackageName);
+  EXPECT_EQ(2u, events.size());
+  EXPECT_TRUE(IsWithinDay(events[0], three_days_prior));
+  EXPECT_TRUE(IsWithinDay(events[1], reference_time));
+
+  // Now add the the other day on the reference date.
+  AppBannerSettingsHelper::RecordCouldShowBannerEvent(
+      web_contents(), url, kTestPackageName, same_day);
+
+  // Now there should still be the same two days.
+  events = AppBannerSettingsHelper::GetCouldShowBannerEvents(
+      web_contents(), url, kTestPackageName);
+  EXPECT_EQ(2u, events.size());
+  EXPECT_TRUE(IsWithinDay(events[0], three_days_prior));
+  EXPECT_TRUE(IsWithinDay(events[1], reference_time));
+}

chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm

@@ -285,6 +285,7 @@ class ContentSettingBubbleWebContentsObserverBridge
     // is implemented
     case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING:
     case CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS:
+    case CONTENT_SETTINGS_TYPE_APP_BANNER:
       NOTREACHED();
   }
   if ((self = [super initWithWindowNibPath:nibPath

chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_unittest.mm

@@ -113,7 +113,8 @@ TEST_F(ContentSettingBubbleControllerTest, Init) {
         i == CONTENT_SETTINGS_TYPE_PPAPI_BROKER ||
         i == CONTENT_SETTINGS_TYPE_MIDI_SYSEX ||
         i == CONTENT_SETTINGS_TYPE_PUSH_MESSAGING ||
-        i == CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS) {
+        i == CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS ||
+        i == CONTENT_SETTINGS_TYPE_APP_BANNER) {
       // These types have no bubble.
       continue;
     }

chrome/browser/ui/webui/options/content_settings_handler.cc

@@ -746,6 +746,11 @@ void ContentSettingsHandler::UpdateExceptionsViewFromModel(
     case CONTENT_SETTINGS_TYPE_METRO_SWITCH_TO_DESKTOP:
       break;
 #endif
+    case CONTENT_SETTINGS_TYPE_APP_BANNER:
+      // The content settings type CONTENT_SETTINGS_TYPE_APP_BANNER is used to
+      // track whether app banners should be shown or not, and is not a user
+      // visible content setting.
+      break;
     default:
       UpdateExceptionsViewFromHostContentSettingsMap(type);
       break;

chrome/chrome_browser.gypi

@@ -33,8 +33,6 @@
       'browser/android/banners/app_banner_manager.h',
       'browser/android/banners/app_banner_metrics_id_list.h',
       'browser/android/banners/app_banner_metrics_ids.h',
-      'browser/android/banners/app_banner_settings_helper.cc',
-      'browser/android/banners/app_banner_settings_helper.h',
       'browser/android/banners/app_banner_utilities.cc',
       'browser/android/banners/app_banner_utilities.h',
       'browser/android/bookmarks/bookmarks_bridge.cc',
@@ -170,6 +168,8 @@
       'browser/app_icon_win.h',
       'browser/app_mode/app_mode_utils.cc',
       'browser/app_mode/app_mode_utils.h',
+      'browser/banners/app_banner_settings_helper.cc',
+      'browser/banners/app_banner_settings_helper.h',
       'browser/bitmap_fetcher/bitmap_fetcher_service.cc',
       'browser/bitmap_fetcher/bitmap_fetcher_service.h',
       'browser/bitmap_fetcher/bitmap_fetcher_service_factory.cc',

chrome/chrome_tests_unit.gypi

@@ -33,6 +33,7 @@
       'browser/autocomplete/shortcuts_provider_unittest.cc',
       'browser/autocomplete/zero_suggest_provider_unittest.cc',
       'browser/autofill/autofill_cc_infobar_delegate_unittest.cc',
+      'browser/banners/app_banner_settings_helper_unittest.cc',
       'browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc',
       'browser/bookmarks/chrome_bookmark_client_unittest.cc',
       'browser/browser_about_handler_unittest.cc',

components/content_settings/core/browser/content_settings_default_provider.cc

@@ -51,9 +51,7 @@ const ContentSetting kDefaultSettings[] = {
 #elif defined(OS_ANDROID) || defined(OS_CHROMEOS)
   CONTENT_SETTING_ASK,      // CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER
 #endif
-#if defined(OS_ANDROID)
   CONTENT_SETTING_DEFAULT,  // CONTENT_SETTINGS_TYPE_APP_BANNER
-#endif
 };
 static_assert(arraysize(kDefaultSettings) == CONTENT_SETTINGS_NUM_TYPES,
               "kDefaultSettings should have CONTENT_SETTINGS_NUM_TYPES "

components/content_settings/core/browser/content_settings_policy_provider.cc

@@ -46,9 +46,7 @@ const char* kPrefToManageType[] = {
 #elif defined(OS_ANDROID) || defined(OS_CHROMEOS)
   NULL,  // No policy for default value of protected media identifier
 #endif
-#if defined(OS_ANDROID)
   NULL,  // No policy for default value of app banners
-#endif
 };
 static_assert(arraysize(kPrefToManageType) == CONTENT_SETTINGS_NUM_TYPES,
               "kPrefToManageType should have CONTENT_SETTINGS_NUM_TYPES "

components/content_settings/core/browser/content_settings_utils.cc

@@ -47,9 +47,7 @@ const char* kTypeNames[] = {
 #elif defined(OS_ANDROID) || defined(OS_CHROMEOS)
   "protected-media-identifier",
 #endif
-#if defined(OS_ANDROID)
   "app-banner",
-#endif
 };
 static_assert(arraysize(kTypeNames) == CONTENT_SETTINGS_NUM_TYPES,
               "kTypeNames should have CONTENT_SETTINGS_NUM_TYPES elements");

components/content_settings/core/browser/host_content_settings_map.cc

@@ -500,11 +500,9 @@ bool HostContentSettingsMap::IsSettingAllowedForType(
     return false;
   }
 
-#if defined(OS_ANDROID)
   // App banners store a dictionary.
   if (content_type == CONTENT_SETTINGS_TYPE_APP_BANNER)
     return false;
-#endif
 
   // DEFAULT, ALLOW and BLOCK are always allowed.
   if (setting == CONTENT_SETTING_DEFAULT ||
@@ -542,10 +540,8 @@ bool HostContentSettingsMap::ContentTypeHasCompoundValue(
   // CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS are of type dictionary/map.
   // Compound types like dictionaries can't be mapped to the type
   // |ContentSetting|.
-#if defined(OS_ANDROID)
   if (type == CONTENT_SETTINGS_TYPE_APP_BANNER)
     return true;
-#endif
 
   return (type == CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE ||
           type == CONTENT_SETTINGS_TYPE_MEDIASTREAM ||

components/content_settings/core/common/content_settings.cc

@@ -64,10 +64,8 @@ ContentSettingsTypeHistogram ContentSettingTypeToHistogramValue(
     case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER:
       return CONTENT_SETTINGS_TYPE_HISTOGRAM_PROTECTED_MEDIA_IDENTIFIER;
 #endif
-#if defined(OS_ANDROID)
     case CONTENT_SETTINGS_TYPE_APP_BANNER:
       return CONTENT_SETTINGS_TYPE_HISTOGRAM_APP_BANNER;
-#endif
     case CONTENT_SETTINGS_NUM_TYPES:
       return CONTENT_SETTINGS_TYPE_HISTOGRAM_INVALID;
   }

components/content_settings/core/common/content_settings_types.h

@@ -46,9 +46,7 @@ enum ContentSettingsType {
 #elif defined(OS_ANDROID) || defined(OS_CHROMEOS)
   CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER,
 #endif
-#if defined(OS_ANDROID)
   CONTENT_SETTINGS_TYPE_APP_BANNER,
-#endif
   CONTENT_SETTINGS_NUM_TYPES,
 };