[App banners] Be less strict about navigations

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 "base/command_line.h"
@@ skipping 27 matching lines @@
 const unsigned int kMinimumBannerBlockedToBannerShown = 90;
 
 // Dictionary keys to use for the events.
 const char* kBannerEventKeys[] = {
     "couldShowBannerEvents",
     "didShowBannerEvent",
     "didBlockBannerEvent",
     "didAddToHomescreenEvent",
 };
 
-// Dictionary key to use whether the banner has been blocked.
-const char kHasBlockedKey[] = "hasBlocked";
-
 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())
@@ skipping 14 matching lines @@
       app_dict = new base::DictionaryValue();
       origin_dict->SetWithoutPathExpansion(key_name, make_scoped_ptr(app_dict));
     }
   }
 
   return app_dict;
 }
 
 }  // namespace
 
+bool AppBannerSettingsHelper::URLsAreForTheSamePage(const GURL& first,
+                                                    const GURL& second) {
+  return first.GetWithEmptyPath() == second.GetWithEmptyPath()
+      && first.path() == second.path() && first.query() == second.query();

[benwells, 2015/03/19 07:18:55]

Formatting nit: the && should be on the previous line.

[gone, 2015/03/19 19:01:35]

Ah, the fun differences between C++ and Java style.

[gone, 2015/03/19 19:01:35]

Done.

[benwells, 2015/03/20 00:44:33]

git cl format is your friend.

+}
+
 void AppBannerSettingsHelper::ClearHistoryForURLs(
     Profile* profile,
     const std::set<GURL>& origin_urls) {
   HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
   for (const GURL& origin_url : origin_urls) {
     ContentSettingsPattern pattern(ContentSettingsPattern::FromURL(origin_url));
     if (!pattern.IsValid())
       continue;
 
     settings->SetWebsiteSetting(pattern, ContentSettingsPattern::Wildcard(),
                                 CONTENT_SETTINGS_TYPE_APP_BANNER, std::string(),
                                 nullptr);
   }
 }
 
 void AppBannerSettingsHelper::RecordBannerEvent(
     content::WebContents* web_contents,
     const GURL& origin_url,
     const std::string& package_name_or_start_url,
     AppBannerEvent event,
     base::Time time) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  if (profile->IsOffTheRecord() || web_contents->GetURL() != origin_url ||
-      package_name_or_start_url.empty()) {
+  if (profile->IsOffTheRecord() || 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;
@@ skipping 168 matching lines @@
   if (!app_dict)
     return base::Time();
 
   std::string event_key(kBannerEventKeys[event]);
   double internal_time;
   if (!app_dict->GetDouble(event_key, &internal_time))
     return base::Time();
 
   return base::Time::FromInternalValue(internal_time);
 }
-
-bool AppBannerSettingsHelper::IsAllowed(

[benwells, 2015/03/19 07:18:55]

Thanks for removing this dead stuff, I'd forgotten about it!

-    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());
-}