Commit f0801207 authored by Maxim Kolosovskiy's avatar Maxim Kolosovskiy Committed by Commit Bot

[Passwords] Use |PwdScriptsFetching| instead of |PwdChangeInSettings|

This CL fixes two issues. Both issues occurred when
|PasswordChangeInSettings| was splitted into two flags
|PasswordChangeInSettings| and |PasswordScriptsFetching|
(https://chromium-review.googlesource.com/c/chromium/src/+/2436339), but
some uses of |PasswordChangeInSettings| were not replaced with
|PasswordScriptsFetching|. The change is supposed to be merged to M87
Beta.

1. The param |scripts_list_url| should be read from
|PasswordScriptsFetching|
Scripts fetching is controlled by |PasswordScriptsFetching| flag, not
|PasswordChangeInSettings| flag. The latter might be disabled while
scripts need to be fetched. So, it makes sense to read the param from
the relevant feature - |PasswordScriptsFetching|.

To be on the safe side, the Finch config can provide the param for
both features, but the param names must be unique even in separate
features. So, this CL renames the param (the param value was not
used yet, so we don't need to update any configs).

2. Scripts prewarming should be controlled by |PasswordScriptsFetching|,
not |PasswordChangeInSettings|.
This issue was not severe and therefore it was hard to notice: even if
scripts were not prefetched in advance, scripts were requested when a
password check page was shown (|PasswordCheckManager::RefreshScripts|).
That method used the correct flag.

Bug: 1095627
Change-Id: Ibcfccf5d1348403b5dd46a15515ce4f040cc8047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2465913
Commit-Queue: Maxim Kolosovskiy  <kolos@chromium.org>
Reviewed-by: default avatarAndrey Zaytsev <andzaytsev@google.com>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816549}
parent b3b2a9ef
......@@ -32,7 +32,7 @@ public class PasswordManagerLauncher {
public static void showPasswordSettings(
Activity activity, @ManagePasswordsReferrer int referrer) {
if (isSyncingPasswordsWithoutCustomPassphrase()
&& ChromeFeatureList.isEnabled(ChromeFeatureList.PASSWORD_CHANGE_IN_SETTINGS)) {
&& ChromeFeatureList.isEnabled(ChromeFeatureList.PASSWORD_SCRIPTS_FETCHING)) {
PasswordScriptsFetcherBridge.prewarmCache();
}
......
......@@ -248,7 +248,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
&omnibox::kOmniboxSuggestionsRecyclerView,
&omnibox::kOmniboxSuggestionsWrapAround,
&password_manager::features::kEditPasswordsInSettings,
&password_manager::features::kPasswordChangeInSettings,
&password_manager::features::kPasswordScriptsFetching,
&password_manager::features::kPasswordCheck,
&password_manager::features::kRecoverFromNeverSaveAndroid,
&performance_hints::features::kContextMenuPerformanceInfo,
......
......@@ -350,8 +350,8 @@ public abstract class ChromeFeatureList {
public static final String PAGE_INFO_PERFORMANCE_HINTS = "PageInfoPerformanceHints";
public static final String PAINT_PREVIEW_DEMO = "PaintPreviewDemo";
public static final String PAINT_PREVIEW_SHOW_ON_STARTUP = "PaintPreviewShowOnStartup";
public static final String PASSWORD_CHANGE_IN_SETTINGS = "PasswordChangeInSettings";
public static final String PASSWORD_CHECK = "PasswordCheck";
public static final String PASSWORD_SCRIPTS_FETCHING = "PasswordScriptsFetching";
public static final String PAY_WITH_GOOGLE_V1 = "PayWithGoogleV1";
public static final String PERMISSION_DELEGATION = "PermissionDelegation";
public static final String PHOTO_PICKER_VIDEO_SUPPORT = "PhotoPickerVideoSupport";
......
......@@ -568,7 +568,7 @@ void ChromePasswordManagerClient::NotifyUserCredentialsWereLeaked(
const base::string16& username) {
#if defined(OS_ANDROID)
if (base::FeatureList::IsEnabled(
password_manager::features::kPasswordChangeInSettings) &&
password_manager::features::kPasswordScriptsFetching) &&
GetPasswordFeatureManager()->IsGenerationEnabled()) {
PasswordScriptsFetcherFactory::GetInstance()
->GetForBrowserContext(web_contents()->GetBrowserContext())
......
......@@ -39,7 +39,7 @@ public class SafetyCheckCoordinator implements DefaultLifecycleObserver {
SigninActivityLauncher signinLauncher) {
new SafetyCheckCoordinator(
settingsFragment, updatesClient, settingsLauncher, signinLauncher);
if (ChromeFeatureList.isEnabled(ChromeFeatureList.PASSWORD_CHANGE_IN_SETTINGS)) {
if (ChromeFeatureList.isEnabled(ChromeFeatureList.PASSWORD_SCRIPTS_FETCHING)) {
// Triggers pre-fetching the list of password change scripts.
PasswordScriptsFetcherBridge.prewarmCache();
}
......
......@@ -95,7 +95,7 @@ constexpr char kDefaultChangePasswordScriptsListUrl[] =
"https://www.gstatic.com/chrome/duplex/change_password_scripts.json";
constexpr base::FeatureParam<std::string> kScriptsListUrlParam{
&features::kPasswordChangeInSettings, "scripts_list_url",
&features::kPasswordScriptsFetching, "custom_list_url",
kDefaultChangePasswordScriptsListUrl};
PasswordScriptsFetcherImpl::PasswordScriptsFetcherImpl(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment