Move website settings fetching from WebsitePreferences to new WebsitePermissionFetcher.

chrome/browser/android/preferences/website_preference_bridge.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 "chrome/browser/android/preferences/website_preference_bridge.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
+#include "base/strings/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
 #include "chrome/browser/browsing_data/cookies_tree_model.h"
 #include "chrome/browser/browsing_data/local_data_container.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
 #include "jni/WebsitePreferenceBridge_jni.h"
 #include "storage/browser/quota/quota_client.h"
 #include "storage/browser/quota/quota_manager.h"
+#include "url/url_constants.h"
 
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaRef;
 using base::android::ScopedJavaGlobalRef;
 using base::android::ScopedJavaLocalRef;
 using content::BrowserThread;
 
 static HostContentSettingsMap* GetHostContentSettingsMap() {
   Profile* profile = ProfileManager::GetActiveUserProfile();
@@ skipping 14 matching lines @@
   for (const auto& settings_it : all_settings) {
     if (settings_it.setting == default_content_setting)
       continue;
     if (managedOnly &&
         HostContentSettingsMap::GetProviderTypeFromSource(settings_it.source) !=
             HostContentSettingsMap::ProviderType::POLICY_PROVIDER) {
       continue;
     }
     const std::string origin = settings_it.primary_pattern.ToString();
     const std::string embedder = settings_it.secondary_pattern.ToString();
-    ScopedJavaLocalRef<jstring> jorigin = ConvertUTF8ToJavaString(env, origin);
+
+    // The string |jorigin| is used to group permissions together in the Site
+    // Settings list. In order to group sites with the same origin, remove any
+    // standard port from the end of the URL if it's present (i.e. remove :443
+    // for HTTPS sites and :80 for HTTP sites).
+    // TODO(sashab,lgarron): Find out which settings are being saved with the
+    // port and omit it if it's the standard port.
+    // TODO(sashab): Remove all this logic and take two passes through
+    // HostContentSettingsMap: once to get all the 'interesting' hosts, and once
+    // (on SingleWebsitePreferences) to find permission patterns which match
+    // each of these hosts.
+    const char* kHttpPortSuffix = ":80";
+    const char* kHttpsPortSuffix = ":443";
+    ScopedJavaLocalRef<jstring> jorigin;
+    if (StartsWithASCII(origin, url::kHttpsScheme, false) &&
+        EndsWith(origin, kHttpsPortSuffix, false)) {
+      jorigin = ConvertUTF8ToJavaString(
+          env, origin.substr(0, origin.size() - strlen(kHttpsPortSuffix)));
+    } else if (StartsWithASCII(origin, url::kHttpScheme, false) &&
+               EndsWith(origin, kHttpPortSuffix, false)) {
+      jorigin = ConvertUTF8ToJavaString(
+          env, origin.substr(0, origin.size() - strlen(kHttpPortSuffix)));
+    } else {
+      jorigin = ConvertUTF8ToJavaString(env, origin);
+    }

[Peter Beverloo, 2015/02/17 15:06:46]

I think we really need Sasha's review on this bit :-(.

Perhaps it's better to split this up - loading and populating the view separated
from unifying the origins of different content settings.

[sashab, 2015/02/18 05:28:32]

Look... To be honest, I think this is the best we can do for now. We should also
ask bauerb@ for some feedback on this origin pattern stuff.

The problem is that the All Sites list needs a list of patterns, not a list of
'sites'. But different permissions save with different patterns, and patterns
for the same site (ie https...:443 and http...:80) really should be grouped.

raymes@ is also working on fixing this, basically by ensuring that all
permissions are saved using consistent patterns. So ideally, in the future, this
fix would no longer be needed since all permissions will share the same pattern.

Long story short: go for it. This is much better than having two entries
(http...(no port) and http...:80) in the list for every site with a permission.

[Michael van Ouwerkerk, 2015/02/18 19:57:39]

Acknowledged.

[Michael van Ouwerkerk, 2015/02/18 19:57:39]

See Sasha's comments. Bernhard also already reviewed this and gave it the ok.

+
     ScopedJavaLocalRef<jstring> jembedder;
     if (embedder != origin)
       jembedder = ConvertUTF8ToJavaString(env, embedder);
     switch (content_type) {
       case CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC:
       case CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA:
         Java_WebsitePreferenceBridge_insertVoiceAndVideoCaptureInfoIntoList(
             env, list, jorigin.obj(), jembedder.obj());
         break;
       case CONTENT_SETTINGS_TYPE_GEOLOCATION:
@@ skipping 547 matching lines @@
   GURL url(ConvertJavaStringToUTF8(env, jorigin));
   scoped_refptr<SiteDataDeleteHelper> site_data_deleter(
       new SiteDataDeleteHelper(profile, url));
   site_data_deleter->Run();
 }
 
 // Register native methods
 bool RegisterWebsitePreferenceBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }