Implement additional parts of the new ui for CBD.

chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
index 82b5794fe09fbd57558e3f0db96f5fc04e46ba7a..54390366c6f53b37c6657c488be8e750e19dbced 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
@@ -238,7 +238,7 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
     // important domains from being cleared.
     private ConfirmImportantSitesDialogFragment mConfirmImportantSitesDialog;
 
-    private final EnumSet<DialogOption> getSelectedOptions() {
+    protected final EnumSet<DialogOption> getSelectedOptions() {

[msramek, 2017/01/25 15:24:23]

Please add a javadoc to methods where you increased visibility.

[dullweber, 2017/01/25 17:22:06]

Done.

         EnumSet<DialogOption> selected = EnumSet.noneOf(DialogOption.class);
         for (Item item : mItems) {
             if (item.isSelected()) selected.add(item.getOption());
@@ -382,18 +382,22 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
     @Override
     public boolean onPreferenceClick(Preference preference) {
         if (preference.getKey().equals(PREF_CLEAR_BUTTON)) {
-            if (shouldShowImportantSitesDialog()) {
-                showImportantDialogThenClear();
-                return true;
-            }
-            // If sites haven't been fetched, just clear the browsing data regularly rather than
-            // waiting to show the important sites dialog.
-            clearBrowsingData(getSelectedOptions(), null, null, null, null);
+            onClearButtonClicked();
             return true;
         }
         return false;
     }
 
+    protected void onClearButtonClicked() {
+        if (shouldShowImportantSitesDialog()) {
+            showImportantDialogThenClear();
+            return;
+        }
+        // If sites haven't been fetched, just clear the browsing data regularly rather than
+        // waiting to show the important sites dialog.
+        clearBrowsingData(getSelectedOptions(), null, null, null, null);
+    }
+
     @Override
     public boolean onPreferenceChange(Preference preference, Object value) {
         if (preference.getKey().equals(PREF_TIME_RANGE)) {
@@ -413,13 +417,17 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
     /**
      * Disable the "Clear" button if none of the options are selected. Otherwise, enable it.
      */
-    private void updateButtonState() {
+    protected void updateButtonState() {
         ButtonPreference clearButton = (ButtonPreference) findPreference(PREF_CLEAR_BUTTON);
         if (clearButton == null) return;
         boolean isEnabled = !getSelectedOptions().isEmpty();
         clearButton.setEnabled(isEnabled);
     }
 
+    protected int getPreferencesXml() {

[msramek, 2017/01/25 15:24:23]

nit: Maybe getLayoutResourceId() would be a better name?

[dullweber, 2017/01/25 17:22:06]

I changed it to getPreferenceXmlId because this is in R.xml.* and not R.layout.*

+        return R.xml.clear_browsing_data_preferences;
+    }
+
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -427,7 +435,7 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
         mMaxImportantSites = PrefServiceBridge.getMaxImportantSites();
         PrefServiceBridge.getInstance().requestInfoAboutOtherFormsOfBrowsingHistory(this);
         getActivity().setTitle(R.string.clear_browsing_data_title);
-        addPreferencesFromResource(R.xml.clear_browsing_data_preferences);
+        addPreferencesFromResource(getPreferencesXml());
         DialogOption[] options = getDialogOptions();
         mItems = new Item[options.length];
         for (int i = 0; i < options.length; i++) {
@@ -475,8 +483,10 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
 
         // The "Clear" button.
         ButtonPreference clearButton = (ButtonPreference) findPreference(PREF_CLEAR_BUTTON);
-        clearButton.setOnPreferenceClickListener(this);
-        clearButton.setShouldDisableView(true);
+        if (clearButton != null) {

[msramek, 2017/01/25 15:24:23]

This is a bit confusing. This class is associated with a XML file which clearly
does have a |clearButton|. If subclasses might not have it, we should address it
via inheritance. Subclasses should define their own onCreate() or helper methods
that will be injected here.

[dullweber, 2017/01/25 17:22:06]

I will look into a good solution for this and the summary texts tomorrow. I also
need to find out how I can integrate a clickable link into the checkbox
preference for history. I guess the code for the texts needs to change as well.

[msramek, 2017/01/26 11:04:07]

As we just discussed offline, we could just extract "initializeClearButton",
"initializeGoogleFooter" etc. I still it is easier to understand if a subclass
@Overrides these methods and empty and comments that "I don't have this footer"
rather than one onCreate() method handling three different screens.

+            clearButton.setOnPreferenceClickListener(this);
+            clearButton.setShouldDisableView(true);
+        }
 
         // The general information footnote informs users about data that will not be deleted.
         // If the user is signed in, it also informs users about the behavior of synced deletions.
@@ -485,33 +495,37 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
         // that they have other forms of browsing history, then also about that.
         TextMessageWithLinkAndIconPreference google_summary =
                 (TextMessageWithLinkAndIconPreference) findPreference(PREF_GOOGLE_SUMMARY);
+        if (google_summary != null) {
+            google_summary.setLinkClickDelegate(new Runnable() {
+                @Override
+                public void run() {
+                    new TabDelegate(false /* incognito */)
+                            .launchUrl(WEB_HISTORY_URL, TabLaunchType.FROM_CHROME_UI);
+                }
+            });
+        }
+
         TextMessageWithLinkAndIconPreference general_summary =
                 (TextMessageWithLinkAndIconPreference) findPreference(PREF_GENERAL_SUMMARY);
-
-        google_summary.setLinkClickDelegate(new Runnable() {
-            @Override
-            public void run() {
-                new TabDelegate(false /* incognito */).launchUrl(
-                        WEB_HISTORY_URL, TabLaunchType.FROM_CHROME_UI);
+        if (general_summary != null) {
+            general_summary.setLinkClickDelegate(new Runnable() {
+                @Override
+                public void run() {
+                    HelpAndFeedback.getInstance(getActivity())
+                            .show(getActivity(), getResources().getString(
+                                                         R.string.help_context_clear_browsing_data),
+                                    Profile.getLastUsedProfile(), null);
+                }
+            });
+            if (ChromeSigninController.get(getActivity()).isSignedIn()) {
+                general_summary.setSummary(
+                        R.string.clear_browsing_data_footnote_sync_and_site_settings);
+            } else {
+                getPreferenceScreen().removePreference(google_summary);
+                general_summary.setSummary(R.string.clear_browsing_data_footnote_site_settings);
             }
-        });
-        general_summary.setLinkClickDelegate(new Runnable() {
-            @Override
-            public void run() {
-                HelpAndFeedback.getInstance(getActivity()).show(
-                        getActivity(),
-                        getResources().getString(R.string.help_context_clear_browsing_data),
-                        Profile.getLastUsedProfile(),
-                        null);
-            }
-        });
-        if (ChromeSigninController.get(getActivity()).isSignedIn()) {
-            general_summary.setSummary(
-                    R.string.clear_browsing_data_footnote_sync_and_site_settings);
-        } else {
-            getPreferenceScreen().removePreference(google_summary);
-            general_summary.setSummary(R.string.clear_browsing_data_footnote_site_settings);
         }
+
         if (ChromeFeatureList.isEnabled(ChromeFeatureList.IMPORTANT_SITES_IN_CBD)) {
             PrefServiceBridge.fetchImportantSites(this);
         }