Move website settings fetching from WebsitePreferences to new WebsitePermissionFetcher.

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
index d337bfa5cfa160abd38b16dbc86a7d8ce70e987a..61c10129699653584b9a365b503b3d0eb05be256 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
@@ -7,6 +7,7 @@
 import android.app.AlertDialog;
 import android.content.Context;
 import android.content.DialogInterface;
+import android.net.Uri;
 import android.os.Bundle;
 import android.preference.ListPreference;
 import android.preference.Preference;
@@ -14,19 +15,31 @@
 import android.preference.Preference.OnPreferenceClickListener;
 import android.preference.PreferenceFragment;
 import android.preference.PreferenceScreen;
+import android.text.TextUtils;
 import android.text.format.Formatter;
 import android.widget.ListAdapter;
 
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ContentSettingsType;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 /**
  * Shows a list of HTML5 settings for a single website.
  */
 public class SingleWebsitePreferences extends PreferenceFragment
         implements DialogInterface.OnClickListener, OnPreferenceChangeListener,
                 OnPreferenceClickListener {
+    // SingleWebsitePreferences expects either EXTRA_SITE (a Website) or
+    // EXTRA_ADDRESS (a WebsiteAddress) to be present (but not both). If
+    // EXTRA_SITE is present, the fragment will display the permissions in that
+    // Website object. If EXTRA_ADDRESS is present, the fragment will find all
+    // permissions for that website address and display those.
     public static final String EXTRA_SITE = "org.chromium.chrome.preferences.site";
+    public static final String EXTRA_ADDRESS = "org.chromium.chrome.preferences.address";
 
     // Preference keys, see single_website_preferences.xml
     // Headings:
@@ -51,13 +64,112 @@
 
     // The website this page is displaying details about.
     private Website mSite;
+
+    // The address of the site we want to display. Used only if EXTRA_ADDRESS is provided.
+    private WebsiteAddress mSiteAddress;
+
     // A list of possible options for each list preference summary.
     private String[] mListPreferenceSummaries;
 
+    private class SingleWebsitePermissionsPopulator

[Miguel Garcia, 2015/02/18 21:40:46]

Let's at least add a TODO for some heavy refactoring here.

[Michael van Ouwerkerk, 2015/02/19 17:19:35]

Done.

+            implements WebsitePermissionsFetcher.WebsitePermissionsCallback {
+        @Override
+        public void onWebsitePermissionsAvailable(
+                Map<String, Set<Website>> sitesByOrigin, Map<String, Set<Website>> sitesByHost) {
+            // This method may be called after the activity has been destroyed.
+            // In that case, bail out.
+            if (getActivity() == null) return;
+
+            // TODO(mvanouwerkerk): Do this at data retrieval time, instead of now.
+            List<Set<Website>> allSites = new ArrayList<>();
+            allSites.addAll(sitesByOrigin.values());
+            allSites.addAll(sitesByHost.values());
+            mSite = mergeWebsiteSettings(mSiteAddress, allSites);
+            displaySitePermissions();
+        }
+    }
+
+    /**
+     * Creates a Bundle with the correct arguments for opening this fragment for
+     * the website with the given url.
+     *
+     * @param url The URL to open the fragment with. This is a complete url including scheme,
+     *            domain, port,  path, etc.
+     * @return The bundle to attach to the preferences intent.
+     */
+    public static Bundle createFragmentArgsForSite(String url) {
+        Bundle fragmentArgs = new Bundle();
+        String origin = getOrigin(url);
+        fragmentArgs.putSerializable(
+                SingleWebsitePreferences.EXTRA_ADDRESS, WebsiteAddress.create(origin));
+        return fragmentArgs;
+    }
+
     @Override
     public void onActivityCreated(Bundle savedInstanceState) {
         getActivity().setTitle(R.string.prefs_content_settings);
-        mSite = (Website) getArguments().getSerializable(EXTRA_SITE);
+        Object extraSite = getArguments().getSerializable(EXTRA_SITE);
+        Object extraAddress = getArguments().getSerializable(EXTRA_ADDRESS);
+
+        if (extraSite != null && extraAddress == null) {
+            mSite = (Website) extraSite;
+            displaySitePermissions();
+        } else if (extraAddress != null && extraSite == null) {
+            mSiteAddress = (WebsiteAddress) extraAddress;
+            WebsitePermissionsFetcher fetcher =
+                    new WebsitePermissionsFetcher(new SingleWebsitePermissionsPopulator());
+            fetcher.fetchAllPreferences();
+        } else {
+            assert false : "Exactly one of EXTRA_SITE or EXTRA_SITE_ADDRESS must be provided.";
+        }
+
+        super.onActivityCreated(savedInstanceState);
+    }
+
+    private static String getOrigin(String url) {
+        Uri uri = Uri.parse(url);

[Miguel Garcia, 2015/02/18 21:40:46]

Would getOriginForDisplay in UrlUtilities.java do the job?

[Michael van Ouwerkerk, 2015/02/19 17:19:34]

Done.

+        String scheme = uri.getScheme();
+        String host = uri.getHost();
+        int port = uri.getPort();
+
+        if (TextUtils.isEmpty(scheme) || TextUtils.isEmpty(host)) {
+            return url;
+        }
+
+        if (port == -1 || (port == 80 && "http".equals(scheme))
+                || (port == 443 && "https".equals(scheme))) {
+            return scheme + "://" + host;
+        }
+        return scheme + "://" + host + ":" + port;
+    }
+
+    /**
+     * Given an address and a list of sets of websites, returns a new site with the same origin

[Miguel Garcia, 2015/02/18 21:40:46]

quadratic complexity with JNI in each call!.

Complexity wise this is O(number of permissions ^2) right? so it's not so
terrible because the number of possible permissions is not very big, perhaps you
can mention it in a comment?
I commented on the JNI piece on C++

[Michael van Ouwerkerk, 2015/02/19 17:19:35]

Done. I've removed the JNI call from it, and added a note about the cost.

+     * as |address| which has merged into it all preferences and settings of the matching input
+     * sites.
+     *
+     * @param address The address to search for.
+     * @param websiteSets The websites to search in.
+     * @return The merged website.
+     */
+    private static Website mergeWebsiteSettings(
+            WebsiteAddress address, List<Set<Website>> websiteSets) {
+        Website mergedSite = new Website(address);
+        for (Set<Website> websiteSet : websiteSets) {
+            for (Website website : websiteSet) {
+                if (address.hasSameContentSettingsOrigin(website.getAddress())) {
+                    mergedSite.merge(website);
+                }
+            }
+        }
+        return mergedSite;
+    }
+
+    /**
+     * Updates the permissions displayed in the UI by fetching them from mSite.
+     * Must only be called once mSite is set.
+     */
+    private void displaySitePermissions() {
         addPreferencesFromResource(R.xml.single_website_preferences);
         mListPreferenceSummaries = getActivity().getResources().getStringArray(
                 R.array.website_settings_permission_options);
@@ -91,11 +203,7 @@ public void onActivityCreated(Bundle savedInstanceState) {
             } else if (PREF_PROTECTED_MEDIA_IDENTIFIER_PERMISSION.equals(preference.getKey())) {
                 setUpListPreference(preference, mSite.getProtectedMediaIdentifierPermission());
             } else if (PREF_PUSH_NOTIFICATIONS_PERMISSION.equals(preference.getKey())) {
-                if (ContentPreferences.pushNotificationsSupported()) {
-                    setUpListPreference(preference, mSite.getPushNotificationPermission());
-                } else {
-                    getPreferenceScreen().removePreference(preference);
-                }
+                setUpListPreference(preference, mSite.getPushNotificationPermission());
             } else if (PREF_VOICE_AND_VIDEO_CAPTURE_PERMISSION.equals(preference.getKey())) {
                 configureVoiceAndVideoPreference(preference);
             }
@@ -111,8 +219,6 @@ public void onActivityCreated(Bundle savedInstanceState) {
             Preference heading = preferenceScreen.findPreference(PREF_PERMISSIONS);
             preferenceScreen.removePreference(heading);
         }
-
-        super.onActivityCreated(savedInstanceState);
     }
 
     private boolean hasUsagePreferences() {