Link to Safe Browsing help page in Safe Browsing settings.

Currently the help button is linked to the general Chrome help center.
We'd like it to be linked to the Safe Browsing help page directly. The
Safe Browsing help page was already added in go/mobilehelprecs.

To reuse more code, create an abstract class named
SafeBrowsingSettingsFragmentBase. This class handles help and menu
related logic. SecuritySettingsFragment,
StandardProtectionSettingsFragment and
EnhancedProtectionSettingsFragment inherit from this base class.

Bug: 1115758
Change-Id: I0fbe28eb21c46e6a0171ea987f1f1f5595111d4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353563Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800698}

chrome/android/java/res/values/ids.xml

@@ -30,14 +30,6 @@
     <item type="id" name="translate_infobar_target_spinner" />
     <item type="id" name="subresource_filter_infobar_toggle" />
 
-    <!-- Menu items IDs in Settings -->
-    <item type="id" name="menu_id_close" />
-    <item type="id" name="menu_id_general_help" />
-    <item type="id" name="menu_id_targeted_help" />
-    <item type="id" name="menu_id_reset" />
-    <item type="id" name="menu_id_refresh" />
-    <item type="id" name="menu_id_settings" />
-
     <!-- Cast notification -->
     <item type="id" name="remote_notification" />
 

chrome/browser/feedback/android/java/res/values/strings.xml

@@ -25,6 +25,7 @@
     <string name="help_context_passwords">passwords</string>
     <string name="help_context_password_leak_detection">leak_detection_android</string>
     <string name="help_context_check_passwords">check_passwords</string>
+    <string name="help_context_safe_browsing">safebrowsing_android</string>
     <!-- TODO(peconn): Add help section. -->
     <!-- <string name="help_context_suggestions">mobile_content_suggestions</string> -->
 </resources>

chrome/browser/safe_browsing/android/BUILD.gn

@@ -27,6 +27,7 @@ android_library("java") {
     "java/src/org/chromium/chrome/browser/safe_browsing/settings/EnhancedProtectionSettingsFragment.java",
     "java/src/org/chromium/chrome/browser/safe_browsing/settings/NoProtectionConfirmationDialog.java",
     "java/src/org/chromium/chrome/browser/safe_browsing/settings/RadioButtonGroupSafeBrowsingPreference.java",
+    "java/src/org/chromium/chrome/browser/safe_browsing/settings/SafeBrowsingSettingsFragmentBase.java",
     "java/src/org/chromium/chrome/browser/safe_browsing/settings/SecuritySettingsFragment.java",
     "java/src/org/chromium/chrome/browser/safe_browsing/settings/StandardProtectionSettingsFragment.java",
   ]
@@ -34,6 +35,7 @@ android_library("java") {
     ":java_resources",
     "//base:base_java",
     "//base:jni_java",
+    "//chrome/browser/feedback/android:java",
     "//chrome/browser/flags:java",
     "//chrome/browser/preferences:java",
     "//chrome/browser/profiles/android:java",
@@ -46,6 +48,7 @@ android_library("java") {
     "//third_party/android_deps:androidx_annotation_annotation_java",
     "//third_party/android_deps:androidx_fragment_fragment_java",
     "//third_party/android_deps:androidx_preference_preference_java",
+    "//third_party/android_deps:androidx_vectordrawable_vectordrawable_java",
     "//ui/android:ui_full_java",
   ]
   annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
@@ -63,6 +66,7 @@ android_library("javatests") {
     ":java",
     "//base:base_java_test_support",
     "//chrome/android:chrome_java",
+    "//chrome/browser/feedback/android:java",
     "//chrome/browser/flags:java",
     "//chrome/browser/preferences:java",
     "//chrome/browser/profiles/android:java",
@@ -91,6 +95,7 @@ android_resources("java_resources") {
     "java/res/xml/standard_protection_preferences.xml",
   ]
   deps = [
+    "//chrome/browser/feedback/android:java_resources",
     "//chrome/browser/ui/android/strings:ui_strings_grd",
     "//components/browser_ui/settings/android:java_resources",
   ]

chrome/browser/safe_browsing/android/java/src/org/chromium/chrome/browser/safe_browsing/settings/EnhancedProtectionSettingsFragment.java

@@ -4,19 +4,12 @@
 
 package org.chromium.chrome.browser.safe_browsing.settings;
 
-import android.os.Bundle;
-
-import androidx.preference.PreferenceFragmentCompat;
-
-import org.chromium.components.browser_ui.settings.SettingsUtils;
-
 /**
  * Fragment containing enhanced protection settings.
  */
-public class EnhancedProtectionSettingsFragment extends PreferenceFragmentCompat {
+public class EnhancedProtectionSettingsFragment extends SafeBrowsingSettingsFragmentBase {
     @Override
-    public void onCreatePreferences(Bundle bundle, String rootKey) {
-        SettingsUtils.addPreferencesFromResource(this, R.xml.enhanced_protection_preferences);
-        getActivity().setTitle(R.string.prefs_section_safe_browsing_title);
+    protected int getPreferenceResource() {
+        return R.xml.enhanced_protection_preferences;
     }
 }

chrome/browser/safe_browsing/android/java/src/org/chromium/chrome/browser/safe_browsing/settings/SafeBrowsingSettingsFragmentBase.java

+// Copyright 2020 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.
+
+package org.chromium.chrome.browser.safe_browsing.settings;
+
+import android.os.Bundle;
+import android.view.Menu;
+import android.view.MenuInflater;
+import android.view.MenuItem;
+
+import androidx.preference.PreferenceFragmentCompat;
+import androidx.vectordrawable.graphics.drawable.VectorDrawableCompat;
+
+import org.chromium.chrome.browser.feedback.FragmentHelpAndFeedbackLauncher;
+import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncher;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.components.browser_ui.settings.SettingsUtils;
+
+/**
+ * The base fragment class for Safe Browsing settings fragments.
+ */
+public abstract class SafeBrowsingSettingsFragmentBase
+        extends PreferenceFragmentCompat implements FragmentHelpAndFeedbackLauncher {
+    private HelpAndFeedbackLauncher mHelpAndFeedbackLauncher;
+
+    @Override
+    public void onCreatePreferences(Bundle bundle, String s) {
+        SettingsUtils.addPreferencesFromResource(this, getPreferenceResource());
+        getActivity().setTitle(R.string.prefs_section_safe_browsing_title);
+
+        onCreatePreferencesInternal(bundle, s);
+
+        setHasOptionsMenu(true);
+    }
+
+    @Override
+    public void setHelpAndFeedbackLauncher(HelpAndFeedbackLauncher helpAndFeedbackLauncher) {
+        mHelpAndFeedbackLauncher = helpAndFeedbackLauncher;
+    }
+
+    @Override
+    public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
+        menu.clear();
+        MenuItem help =
+                menu.add(Menu.NONE, R.id.menu_id_targeted_help, Menu.NONE, R.string.menu_help);
+        help.setIcon(VectorDrawableCompat.create(
+                getResources(), R.drawable.ic_help_and_feedback, getActivity().getTheme()));
+    }
+
+    @Override
+    public boolean onOptionsItemSelected(MenuItem item) {
+        if (item.getItemId() != R.id.menu_id_targeted_help) {
+            return false;
+        }
+        mHelpAndFeedbackLauncher.show(getActivity(), getString(R.string.help_context_safe_browsing),
+                Profile.getLastUsedRegularProfile(), null);
+        return true;
+    }
+
+    /**
+     * Called within {@link SafeBrowsingSettingsFragmentBase#onCreatePreferences(Bundle, String)}.
+     * If the child class needs to handle specific logic during preference creation, it can override
+     * this method.
+     */
+    protected void onCreatePreferencesInternal(Bundle bundle, String s) {}
+
+    /**
+     * @return The resource id of the preference.
+     */
+    protected abstract int getPreferenceResource();
+}

chrome/browser/safe_browsing/android/java/src/org/chromium/chrome/browser/safe_browsing/settings/SecuritySettingsFragment.java

@@ -9,7 +9,6 @@ import android.os.Bundle;
 
 import androidx.annotation.VisibleForTesting;
 import androidx.preference.Preference;
-import androidx.preference.PreferenceFragmentCompat;
 
 import org.chromium.base.IntentUtils;
 import org.chromium.base.metrics.RecordHistogram;
@@ -22,14 +21,13 @@ import org.chromium.chrome.browser.settings.ChromeManagedPreferenceDelegate;
 import org.chromium.chrome.browser.settings.FragmentSettingsLauncher;
 import org.chromium.chrome.browser.settings.SettingsLauncher;
 import org.chromium.components.browser_ui.settings.ManagedPreferenceDelegate;
-import org.chromium.components.browser_ui.settings.SettingsUtils;
 import org.chromium.components.browser_ui.settings.TextMessagePreference;
 
 /**
  * Fragment containing security settings.
  * TODO(crbug.com/1097310): Rename it to SafeBrowsingSettingsFragment.
  */
-public class SecuritySettingsFragment extends PreferenceFragmentCompat
+public class SecuritySettingsFragment extends SafeBrowsingSettingsFragmentBase
         implements FragmentSettingsLauncher,
                    RadioButtonGroupSafeBrowsingPreference.OnSafeBrowsingModeDetailsRequested,
                    Preference.OnPreferenceChangeListener {
@@ -76,10 +74,7 @@ public class SecuritySettingsFragment extends PreferenceFragmentCompat
     }
 
     @Override
-    public void onCreatePreferences(Bundle bundle, String s) {
-        SettingsUtils.addPreferencesFromResource(this, R.xml.security_preferences);
-        getActivity().setTitle(R.string.prefs_safe_browsing_title);
-
+    protected void onCreatePreferencesInternal(Bundle bundle, String s) {
         mAccessPoint =
                 IntentUtils.safeGetInt(getArguments(), ACCESS_POINT, SettingsAccessPoint.DEFAULT);
 
@@ -101,6 +96,11 @@ public class SecuritySettingsFragment extends PreferenceFragmentCompat
         recordUserActionHistogram(UserAction.SHOWED);
     }
 
+    @Override
+    protected int getPreferenceResource() {
+        return R.xml.security_preferences;
+    }
+
     @Override
     public void onSafeBrowsingModeDetailsRequested(@SafeBrowsingState int safeBrowsingState) {
         recordUserActionHistogramForStateDetailsClicked(safeBrowsingState);

chrome/browser/safe_browsing/android/java/src/org/chromium/chrome/browser/safe_browsing/settings/StandardProtectionSettingsFragment.java

@@ -8,7 +8,6 @@ import android.os.Bundle;
 
 import androidx.annotation.VisibleForTesting;
 import androidx.preference.Preference;
-import androidx.preference.PreferenceFragmentCompat;
 
 import org.chromium.chrome.browser.preferences.Pref;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -17,7 +16,6 @@ import org.chromium.chrome.browser.safe_browsing.SafeBrowsingState;
 import org.chromium.chrome.browser.settings.ChromeManagedPreferenceDelegate;
 import org.chromium.components.browser_ui.settings.ChromeSwitchPreference;
 import org.chromium.components.browser_ui.settings.ManagedPreferenceDelegate;
-import org.chromium.components.browser_ui.settings.SettingsUtils;
 import org.chromium.components.prefs.PrefService;
 import org.chromium.components.user_prefs.UserPrefs;
 
@@ -25,7 +23,7 @@ import org.chromium.components.user_prefs.UserPrefs;
  * Fragment containing standard protection settings.
  */
 public class StandardProtectionSettingsFragment
-        extends PreferenceFragmentCompat implements Preference.OnPreferenceChangeListener {
+        extends SafeBrowsingSettingsFragmentBase implements Preference.OnPreferenceChangeListener {
     @VisibleForTesting
     static final String PREF_EXTENDED_REPORTING = "extended_reporting";
     @VisibleForTesting
@@ -39,10 +37,7 @@ public class StandardProtectionSettingsFragment
     private final PrefService mPrefService = UserPrefs.get(Profile.getLastUsedRegularProfile());
 
     @Override
-    public void onCreatePreferences(Bundle bundle, String rootKey) {
-        SettingsUtils.addPreferencesFromResource(this, R.xml.standard_protection_preferences);
-        getActivity().setTitle(R.string.prefs_section_safe_browsing_title);
-
+    protected void onCreatePreferencesInternal(Bundle bundle, String rootKey) {
         mExtendedReportingPreference = findPreference(PREF_EXTENDED_REPORTING);
         mExtendedReportingPreference.setOnPreferenceChangeListener(this);
         // TODO(crbug.com/1108604): Set ManagedPreferenceDelegate for mExtendedReportingPreference
@@ -55,6 +50,11 @@ public class StandardProtectionSettingsFragment
         updateLeakDetectionAndExtendedReportingPreferences();
     }
 
+    @Override
+    protected int getPreferenceResource() {
+        return R.xml.standard_protection_preferences;
+    }
+
     /**
      * Update the appearance of the preferences under this fragment. The setEnabled function sets
      * whether the toggle is clickable. The setChecked function sets whether the toggle is currently

chrome/browser/safe_browsing/android/javatests/src/org/chromium/chrome/browser/safe_browsing/settings/SecuritySettingsFragmentTest.java

@@ -9,6 +9,7 @@ import static androidx.test.espresso.action.ViewActions.click;
 import static androidx.test.espresso.assertion.ViewAssertions.doesNotExist;
 import static androidx.test.espresso.assertion.ViewAssertions.matches;
 import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
+import static androidx.test.espresso.matcher.ViewMatchers.withId;
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
 
 import androidx.test.filters.SmallTest;
@@ -23,8 +24,10 @@ import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
 import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncher;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
+import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.safe_browsing.SafeBrowsingBridge;
 import org.chromium.chrome.browser.safe_browsing.SafeBrowsingState;
 import org.chromium.chrome.browser.settings.SettingsActivityTestRule;
@@ -59,6 +62,9 @@ public class SecuritySettingsFragmentTest {
     @Mock
     private SettingsLauncher mSettingsLauncher;
 
+    @Mock
+    private HelpAndFeedbackLauncher mHelpAndFeedbackLauncher;
+
     private SecuritySettingsFragment mSecuritySettingsFragment;
     private RadioButtonGroupSafeBrowsingPreference mSafeBrowsingPreference;
     private TextMessagePreference mManagedTextPreference;
@@ -297,6 +303,22 @@ public class SecuritySettingsFragmentTest {
         });
     }
 
+    @Test
+    @SmallTest
+    @Feature({"SafeBrowsing"})
+    public void testHelpButtonClicked() {
+        launchSettingsActivity();
+        mSecuritySettingsFragment.setHelpAndFeedbackLauncher(mHelpAndFeedbackLauncher);
+        onView(withId(R.id.menu_id_targeted_help)).perform(click());
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            Mockito.verify(mHelpAndFeedbackLauncher)
+                    .show(mSecuritySettingsFragment.getActivity(),
+                            mSecuritySettingsFragment.getString(
+                                    R.string.help_context_safe_browsing),
+                            Profile.getLastUsedRegularProfile(), null);
+        });
+    }
+
     private @SafeBrowsingState int getSafeBrowsingState() {
         return mSafeBrowsingPreference.getSafeBrowsingStateForTesting();
     }

components/browser_ui/settings/android/BUILD.gn

@@ -58,6 +58,7 @@ android_resources("java_resources") {
     "java/res/layout/preference_spinner_single_line_item.xml",
     "java/res/layout/settings_action_bar_shadow.xml",
     "java/res/values/attrs.xml",
+    "java/res/values/ids.xml",
     "java/res/values/styles.xml",
   ]
 

components/browser_ui/settings/android/java/res/values/ids.xml

+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2020 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.
+-->
+<resources>
+    <!-- Menu items IDs in Settings -->
+    <item type="id" name="menu_id_general_help" />
+    <item type="id" name="menu_id_targeted_help" />
+</resources>