Fix notification settings for file scheme

The DesktopNotificationService gets empty GURLs in case of file scheme urls. This CL deals with the that to fix breaking regression tests. The root cause of the problem remains to be fixed.

BUG=76693
TEST=desktop_notification_service_unittest.cc

Review URL: http://codereview.chromium.org/6719021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@79115 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 117 insertions, 24 deletions

diff --git a/chrome/browser/content_settings/content_settings_notification_provider.cc b/chrome/browser/content_settings/content_settings_notification_provider.cc
index 34244f7..db0019e 100644
--- a/chrome/browser/content_settings/content_settings_notification_provider.cc
+++ b/chrome/browser/content_settings/content_settings_notification_provider.cc
@@ -5,6 +5,7 @@
 
 #include "chrome/browser/content_settings/content_settings_notification_provider.h"
 
+#include "base/string_util.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notifications_prefs_cache.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
@@ -13,13 +14,13 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/content_settings_types.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
 #include "content/common/notification_service.h"
 #include "content/common/notification_type.h"
 #include "googleurl/src/gurl.h"
 
 namespace {
 
-const std::string HTTP_PREFIX("http://");
 const ContentSetting kDefaultSetting = CONTENT_SETTING_ASK;
 
 }  // namespace
@@ -38,6 +39,45 @@ void NotificationProvider::RegisterUserPrefs(PrefService* user_prefs) {
     user_prefs->RegisterListPref(prefs::kDesktopNotificationDeniedOrigins);
 }
 
+// TODO(markusheintz): Re-factoring in progress. Do not move or touch the
+// following two static methods as you might cause trouble. Thanks!
+
+// static
+ContentSettingsPattern NotificationProvider::ToContentSettingsPattern(
+    const GURL& origin) {
+  // Fix empty GURLs.
+  if (origin.spec().empty()) {
+    std::string pattern_spec(chrome::kFileScheme);
+    pattern_spec += chrome::kStandardSchemeSeparator;
+    return ContentSettingsPattern(pattern_spec);
+  }
+  return ContentSettingsPattern::FromURLNoWildcard(origin);
+}
+
+// static
+GURL NotificationProvider::ToGURL(const ContentSettingsPattern& pattern) {
+  std::string pattern_spec(pattern.AsString());
+
+  if (pattern_spec.empty() ||
+      StartsWithASCII(pattern_spec,
+                      std::string(ContentSettingsPattern::kDomainWildcard),
+                      true)) {
+    NOTREACHED();
+  }
+
+  std::string url_spec("");
+  if (StartsWithASCII(pattern_spec, std::string(chrome::kFileScheme), false)) {
+    url_spec += pattern_spec;
+  } else if (!pattern.scheme().empty()) {
+    url_spec += pattern.scheme();
+    url_spec += chrome::kStandardSchemeSeparator;
+    url_spec += pattern_spec;
+  }
+
+  LOG(ERROR) << " url_spec=" << url_spec;
+  return GURL(url_spec);
+}
+
 NotificationProvider::NotificationProvider(
     Profile* profile)
     : profile_(profile) {
@@ -74,13 +114,7 @@ void NotificationProvider::SetContentSetting(
   if (content_type != CONTENT_SETTINGS_TYPE_NOTIFICATIONS)
     return;
 
-  std::string origin_str(requesting_url_pattern.AsString());
-
-  if (0 == origin_str.find_first_of(ContentSettingsPattern::kDomainWildcard)) {
-    NOTREACHED();
-  }
-
-  GURL origin(HTTP_PREFIX + origin_str);
+  GURL origin = ToGURL(requesting_url_pattern);
   if (CONTENT_SETTING_ALLOW == content_setting) {
     GrantPermission(origin);
   } else if (CONTENT_SETTING_BLOCK == content_setting) {
@@ -241,7 +275,6 @@ void NotificationProvider::PersistPermissionChange(
   // Don't persist changes when off the record.
   if (profile_->IsOffTheRecord())
     return;
-
   PrefService* prefs = profile_->GetPrefs();
 
   // |Observe()| updates the whole permission set in the cache, but only a
@@ -300,6 +333,7 @@ void NotificationProvider::PersistPermissionChange(
 ContentSetting NotificationProvider::GetContentSetting(
     const GURL& origin) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   if (profile_->IsOffTheRecord())
     return kDefaultSetting;
 
diff --git a/chrome/browser/content_settings/content_settings_notification_provider.h b/chrome/browser/content_settings/content_settings_notification_provider.h
index 5680464..74fcf8c 100644
--- a/chrome/browser/content_settings/content_settings_notification_provider.h
+++ b/chrome/browser/content_settings/content_settings_notification_provider.h
@@ -26,6 +26,10 @@ class NotificationProvider : public ProviderInterface,
  public:
   static void RegisterUserPrefs(PrefService* user_prefs);
 
+  static ContentSettingsPattern ToContentSettingsPattern(const GURL& origin);
+
+  static GURL ToGURL(const ContentSettingsPattern& pattern);
+
   explicit NotificationProvider(Profile* profile);
 
   virtual ~NotificationProvider();
diff --git a/chrome/browser/content_settings/content_settings_pattern.cc b/chrome/browser/content_settings/content_settings_pattern.cc
index ef759a8..8a52a1d 100644
--- a/chrome/browser/content_settings/content_settings_pattern.cc
+++ b/chrome/browser/content_settings/content_settings_pattern.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -11,12 +11,14 @@
 #include "googleurl/src/url_canon.h"
 
 namespace {
+
 bool IsValidHostlessPattern(const std::string& pattern) {
   std::string file_scheme_plus_separator(chrome::kFileScheme);
   file_scheme_plus_separator += chrome::kStandardSchemeSeparator;
 
   return StartsWithASCII(pattern, file_scheme_plus_separator, false);
 }
+
 }  // namespace
 
 // The version of the pattern format implemented. Version 1 includes the
@@ -35,6 +37,7 @@ const size_t ContentSettingsPattern::kDomainWildcardLength = 4;
 // static
 ContentSettingsPattern ContentSettingsPattern::FromURL(
     const GURL& url) {
+  // TODO(markusheintz): Add scheme wildcard;
   return ContentSettingsPattern(!url.has_host() || url.HostIsIPAddress() ?
       net::GetHostOrSpecFromURL(url) :
       std::string(kDomainWildcard) + url.host());
@@ -43,7 +46,7 @@ ContentSettingsPattern ContentSettingsPattern::FromURL(
 // static
 ContentSettingsPattern ContentSettingsPattern::FromURLNoWildcard(
     const GURL& url) {
-  return ContentSettingsPattern(net::GetHostOrSpecFromURL(url));
+  return ContentSettingsPattern(net::GetHostOrSpecFromURL(url), url.scheme());
 }
 
 bool ContentSettingsPattern::IsValid() const {
diff --git a/chrome/browser/content_settings/content_settings_pattern.h b/chrome/browser/content_settings/content_settings_pattern.h
index 344d31e..a9440c5 100644
--- a/chrome/browser/content_settings/content_settings_pattern.h
+++ b/chrome/browser/content_settings/content_settings_pattern.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -26,7 +26,8 @@ class ContentSettingsPattern {
   ContentSettingsPattern() {}
 
   explicit ContentSettingsPattern(const std::string& pattern)
-      : pattern_(pattern) {}
+      : pattern_(pattern),
+        scheme_("") {}
 
   // True if this is a valid pattern. Valid patterns are
   //   - [*.]domain.tld (matches domain.tld and all sub-domains)
@@ -50,6 +51,10 @@ class ContentSettingsPattern {
   // in original (if it was already ASCII) or punycode form.
   std::string CanonicalizePattern() const;
 
+  std::string scheme() const {
+    return scheme_;
+  }
+
   // The version of the pattern format implemented.
   static const int kContentSettingsPatternVersion;
 
@@ -60,7 +65,19 @@ class ContentSettingsPattern {
   static const size_t kDomainWildcardLength;
 
  private:
+  // TODO(markusheintz): This constructor is only here to fix bug 76693. Further
+  // refactoring pending to fully integrate scheme support in content settings
+  // patterns.
+  ContentSettingsPattern(const std::string& host, const std::string& scheme)
+      : pattern_(host),
+        scheme_(scheme) {}
+
   std::string pattern_;
+
+  // TODO(markusheintz): This is only here to fix bug 76693. There is more work
+  // to do to add scheme support to content-settings patterns.
+  // TODO(markusheintz): canonicalize to lowercase;
+  std::string scheme_;
 };
 
 // Stream operator so ContentSettingsPattern can be used in assertion
diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc
index e372055..a6e4eb5 100644
--- a/chrome/browser/notifications/desktop_notification_service.cc
+++ b/chrome/browser/notifications/desktop_notification_service.cc
@@ -45,8 +45,6 @@ const ContentSetting kDefaultSetting = CONTENT_SETTING_ASK;
 
 namespace {
 
-const std::string HTTP_PREFIX("http://");
-
 typedef content_settings::ProviderInterface::Rules Rules;
 
 void GetOriginsWithSettingFromContentSettingsRules(
@@ -64,17 +62,19 @@ void GetOriginsWithSettingFromContentSettingsRules(
         // TODO(markusheintz): This will be removed in one of the next
         // refactoring steps as this entire function will disapear.
         LOG(DFATAL) << "Ignoring invalid content settings pattern: "
-                   << url_str;
+                    << url_str;
       } else if (url_str.find(ContentSettingsPattern::kDomainWildcard) == 0) {
         // TODO(markusheintz): This must be changed once the UI code is
         // refactored and content_settings patterns are fully supported for
         // notifications settings.
         LOG(DFATAL) << "Ignoring unsupported content settings pattern: "
-                   << url_str << ". Content settings patterns other than "
-                   << "hostnames (e.g. wildcard patterns) are not supported "
-                   << "for notification content settings yet.";
+                    << url_str << ". Content settings patterns other than "
+                    << "hostnames (e.g. wildcard patterns) are not supported "
+                    << "for notification content settings yet.";
       } else {
-        origins->push_back(GURL(HTTP_PREFIX + url_str));
+        origins->push_back(
+            content_settings::NotificationProvider::ToGURL(
+                rule->requesting_url_pattern));
       }
     }
   }
@@ -318,7 +318,7 @@ void DesktopNotificationService::StopObserving() {
 
 void DesktopNotificationService::GrantPermission(const GURL& origin) {
   ContentSettingsPattern pattern =
-      ContentSettingsPattern::FromURLNoWildcard(origin);
+      content_settings::NotificationProvider::ToContentSettingsPattern(origin);
   provider_->SetContentSetting(
       pattern,
       pattern,
@@ -335,11 +335,10 @@ void DesktopNotificationService::GrantPermission(const GURL& origin) {
           origin));
 }
 
-
 void DesktopNotificationService::DenyPermission(const GURL& origin) {
   // Update content settings
   ContentSettingsPattern pattern =
-      ContentSettingsPattern::FromURLNoWildcard(origin);
+      content_settings::NotificationProvider::ToContentSettingsPattern(origin);
   provider_->SetContentSetting(
       pattern,
       pattern,
diff --git a/chrome/browser/notifications/desktop_notification_service_unittest.cc b/chrome/browser/notifications/desktop_notification_service_unittest.cc
index 7b4a28e..3d9af1a 100644
--- a/chrome/browser/notifications/desktop_notification_service_unittest.cc
+++ b/chrome/browser/notifications/desktop_notification_service_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -137,6 +137,42 @@ TEST_F(DesktopNotificationServiceTest, DefaultContentSettingSentToCache) {
   EXPECT_EQ(CONTENT_SETTING_BLOCK, cache_->CachedDefaultContentSetting());
 }
 
+TEST_F(DesktopNotificationServiceTest, SettingsForSchemes) {
+  GURL url("file:///html/test.html");
+
+  EXPECT_EQ(CONTENT_SETTING_ASK, cache_->CachedDefaultContentSetting());
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionNotAllowed,
+            proxy_->CacheHasPermission(cache_, url));
+
+  service_->GrantPermission(url);
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionAllowed,
+            proxy_->CacheHasPermission(cache_, url));
+
+  service_->DenyPermission(url);
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionDenied,
+            proxy_->CacheHasPermission(cache_, url));
+
+  GURL https_url("https://testurl");
+  GURL http_url("http://testurl");
+  EXPECT_EQ(CONTENT_SETTING_ASK, cache_->CachedDefaultContentSetting());
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionNotAllowed,
+            proxy_->CacheHasPermission(cache_, http_url));
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionNotAllowed,
+            proxy_->CacheHasPermission(cache_, https_url));
+
+  service_->GrantPermission(https_url);
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionAllowed,
+            proxy_->CacheHasPermission(cache_, https_url));
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionNotAllowed,
+            proxy_->CacheHasPermission(cache_, http_url));
+
+  service_->DenyPermission(http_url);
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionDenied,
+            proxy_->CacheHasPermission(cache_, http_url));
+  EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionAllowed,
+            proxy_->CacheHasPermission(cache_, https_url));
+}
+
 TEST_F(DesktopNotificationServiceTest, GrantPermissionSentToCache) {
   GURL url("http://allowed.com");
   EXPECT_EQ(WebKit::WebNotificationPresenter::PermissionNotAllowed,