Commit bb4a2135 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Conditionally link to Google Password Manager in Settings

This change modifies the "Passwords" entry in the settings page to link
to the Google Password Manager if the necessary conditions are met.
Currently this includes users that are syncing their passwords without
a custom passphrase and for which the corresponding Google Password
Manager experiment is active.

Bug: 904821
Change-Id: If93ee671b1c1b9aee17467aa9382fe5e834ead55
Reviewed-on: https://chromium-review.googlesource.com/c/1350877
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612231}
parent 37636f09
...@@ -19,6 +19,7 @@ js_type_check("closure_compile") { ...@@ -19,6 +19,7 @@ js_type_check("closure_compile") {
":lock_state_behavior", ":lock_state_behavior",
":manage_profile", ":manage_profile",
":manage_profile_browser_proxy", ":manage_profile_browser_proxy",
":people_browser_proxy",
":people_page", ":people_page",
":profile_info_browser_proxy", ":profile_info_browser_proxy",
":setup_fingerprint_dialog", ":setup_fingerprint_dialog",
...@@ -150,10 +151,17 @@ js_library("lock_screen_password_prompt_dialog") { ...@@ -150,10 +151,17 @@ js_library("lock_screen_password_prompt_dialog") {
] ]
} }
js_library("people_browser_proxy") {
deps = [
"//ui/webui/resources/js:cr",
]
}
js_library("people_page") { js_library("people_page") {
deps = [ deps = [
":lock_screen", ":lock_screen",
":lock_state_behavior", ":lock_state_behavior",
":people_browser_proxy",
":profile_info_browser_proxy", ":profile_info_browser_proxy",
":signout_dialog", ":signout_dialog",
":sync_browser_proxy", ":sync_browser_proxy",
......
<link rel="import" href="chrome://resources/html/cr.html">
<script src="people_browser_proxy.js"></script>
// Copyright 2018 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.
/**
* @fileoverview A helper object used from the People section to interact with
* the browser.
*/
cr.exportPath('settings');
cr.define('settings', function() {
/** @interface */
class PeopleBrowserProxy {
// TODO(dpapad): Create a simple OpenWindowProxy class that replaces the
// need for this method and similar methods in other BrowserProxy classes.
/**
* Opens the specified URL in a new tab.
* @param {string} url
*/
openURL(url) {}
}
/** @implements {settings.PeopleBrowserProxy} */
class PeopleBrowserProxyImpl {
/** @override */
openURL(url) {
window.open(url);
}
}
cr.addSingletonGetter(PeopleBrowserProxyImpl);
return {
PeopleBrowserProxy: PeopleBrowserProxy,
PeopleBrowserProxyImpl: PeopleBrowserProxyImpl,
};
});
...@@ -12,9 +12,10 @@ ...@@ -12,9 +12,10 @@
<link rel="import" href="chrome://resources/polymer/v1_0/paper-button/paper-button.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-button/paper-button.html">
<link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
<link rel="import" href="../controls/settings_toggle_button.html"> <link rel="import" href="../controls/settings_toggle_button.html">
<link rel="import" href="sync_page.html"> <link rel="import" href="people_browser_proxy.html">
<link rel="import" href="profile_info_browser_proxy.html"> <link rel="import" href="profile_info_browser_proxy.html">
<link rel="import" href="sync_browser_proxy.html"> <link rel="import" href="sync_browser_proxy.html">
<link rel="import" href="sync_page.html">
<link rel="import" href="../icons.html"> <link rel="import" href="../icons.html">
<link rel="import" href="../route.html"> <link rel="import" href="../route.html">
<link rel="import" href="../settings_page/settings_animated_pages.html"> <link rel="import" href="../settings_page/settings_animated_pages.html">
......
...@@ -311,12 +311,16 @@ Polymer({ ...@@ -311,12 +311,16 @@ Polymer({
}, },
/** /**
* Shows the manage passwords sub page. * Shows a page to manage passwords. This is either the passwords sub page or
* the Google Password Manager page.
* @param {!Event} event * @param {!Event} event
* @private * @private
*/ */
onPasswordsTap_: function(event) { onPasswordsTap_: function(event) {
settings.navigateTo(settings.routes.MANAGE_PASSWORDS); loadTimeData.getBoolean('navigateToGooglePasswordManager') ?
settings.PeopleBrowserProxyImpl.getInstance().openURL(
loadTimeData.getString('googlePasswordManagerUrl')) :
settings.navigateTo(settings.routes.MANAGE_PASSWORDS);
}, },
/** /**
......
...@@ -763,6 +763,12 @@ ...@@ -763,6 +763,12 @@
<structure name="IDR_SETTINGS_PAYMENTS_SECTION_JS" <structure name="IDR_SETTINGS_PAYMENTS_SECTION_JS"
file="passwords_and_forms_page/payments_section.js" file="passwords_and_forms_page/payments_section.js"
type="chrome_html" /> type="chrome_html" />
<structure name="IDR_SETTINGS_PEOPLE_BROWSER_PROXY_HTML"
file="people_page/people_browser_proxy.html"
type="chrome_html" />
<structure name="IDR_SETTINGS_PEOPLE_BROWSER_PROXY_JS"
file="people_page/people_browser_proxy.js"
type="chrome_html" />
<structure name="IDR_SETTINGS_PEOPLE_PAGE_HTML" <structure name="IDR_SETTINGS_PEOPLE_PAGE_HTML"
file="people_page/people_page.html" file="people_page/people_page.html"
type="chrome_html" type="chrome_html"
......
...@@ -190,6 +190,14 @@ GURL GetGooglePasswordManagerURL(ManagePasswordsReferrer referrer) { ...@@ -190,6 +190,14 @@ GURL GetGooglePasswordManagerURL(ManagePasswordsReferrer referrer) {
return net::AppendQueryParameter(url, "utm_campaign", campaign); return net::AppendQueryParameter(url, "utm_campaign", campaign);
} }
bool ShouldManagePasswordsinGooglePasswordManager(Profile* profile) {
return base::FeatureList::IsEnabled(
password_manager::features::kGooglePasswordManager) &&
password_manager_util::GetPasswordSyncState(
ProfileSyncServiceFactory::GetForProfile(profile)) ==
password_manager::SYNCING_NORMAL_ENCRYPTION;
}
// Navigation is handled differently on Android. // Navigation is handled differently on Android.
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
void NavigateToGooglePasswordManager(Profile* profile, void NavigateToGooglePasswordManager(Profile* profile,
...@@ -202,11 +210,7 @@ void NavigateToGooglePasswordManager(Profile* profile, ...@@ -202,11 +210,7 @@ void NavigateToGooglePasswordManager(Profile* profile,
void NavigateToManagePasswordsPage(Browser* browser, void NavigateToManagePasswordsPage(Browser* browser,
ManagePasswordsReferrer referrer) { ManagePasswordsReferrer referrer) {
if (base::FeatureList::IsEnabled( if (ShouldManagePasswordsinGooglePasswordManager(browser->profile())) {
password_manager::features::kGooglePasswordManager) &&
password_manager_util::GetPasswordSyncState(
ProfileSyncServiceFactory::GetForProfile(browser->profile())) ==
password_manager::SYNCING_NORMAL_ENCRYPTION) {
NavigateToGooglePasswordManager(browser->profile(), referrer); NavigateToGooglePasswordManager(browser->profile(), referrer);
} else { } else {
chrome::ShowPasswordManager(browser); chrome::ShowPasswordManager(browser);
......
...@@ -79,6 +79,13 @@ bool IsSyncingAutosignSetting(Profile* profile); ...@@ -79,6 +79,13 @@ bool IsSyncingAutosignSetting(Profile* profile);
GURL GetGooglePasswordManagerURL( GURL GetGooglePasswordManagerURL(
password_manager::ManagePasswordsReferrer referrer); password_manager::ManagePasswordsReferrer referrer);
// Returns whether users should manage their passwords in the Google Password
// Manager. This includes users that are syncing their passwords without a
// custom passphrase and for which the Google Password Manager experiment is
// activated. For these users links to the Chrome password settings will be
// repaced with links to the Google Password Manager, i.e. passwords.google.com.
bool ShouldManagePasswordsinGooglePasswordManager(Profile* profile);
// Navigates to the Google Password Manager, i.e. passwords.google.com. // Navigates to the Google Password Manager, i.e. passwords.google.com.
void NavigateToGooglePasswordManager( void NavigateToGooglePasswordManager(
Profile* profile, Profile* profile,
......
...@@ -1500,14 +1500,15 @@ void AddPasswordsAndFormsStrings(content::WebUIDataSource* html_source, ...@@ -1500,14 +1500,15 @@ void AddPasswordsAndFormsStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_CREDIT_CARD_NONE); IDS_SETTINGS_CREDIT_CARD_NONE);
} }
GURL google_password_manager_url = GetGooglePasswordManagerURL(
password_manager::ManagePasswordsReferrer::kChromeSettings);
html_source->AddString( html_source->AddString(
"managePasswordsLabel", "managePasswordsLabel",
l10n_util::GetStringFUTF16( l10n_util::GetStringFUTF16(
IDS_SETTINGS_PASSWORDS_MANAGE_PASSWORDS, IDS_SETTINGS_PASSWORDS_MANAGE_PASSWORDS,
base::UTF8ToUTF16( base::UTF8ToUTF16(google_password_manager_url.spec())));
GetGooglePasswordManagerURL( html_source->AddString("googlePasswordManagerUrl",
password_manager::ManagePasswordsReferrer::kChromeSettings) google_password_manager_url.spec());
.spec())));
html_source->AddString("passwordManagerLearnMoreURL", html_source->AddString("passwordManagerLearnMoreURL",
chrome::kPasswordManagerLearnMoreURL); chrome::kPasswordManagerLearnMoreURL);
html_source->AddString("manageAddressesUrl", html_source->AddString("manageAddressesUrl",
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/passwords/manage_passwords_view_utils.h"
#include "chrome/browser/ui/webui/metrics_handler.h" #include "chrome/browser/ui/webui/metrics_handler.h"
#include "chrome/browser/ui/webui/settings/about_handler.h" #include "chrome/browser/ui/webui/settings/about_handler.h"
#include "chrome/browser/ui/webui/settings/appearance_handler.h" #include "chrome/browser/ui/webui/settings/appearance_handler.h"
...@@ -368,6 +369,10 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui) ...@@ -368,6 +369,10 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui)
"autofillHomeEnabled", "autofillHomeEnabled",
base::FeatureList::IsEnabled(password_manager::features::kAutofillHome)); base::FeatureList::IsEnabled(password_manager::features::kAutofillHome));
html_source->AddBoolean(
"navigateToGooglePasswordManager",
ShouldManagePasswordsinGooglePasswordManager(profile));
html_source->AddBoolean("showImportPasswords", html_source->AddBoolean("showImportPasswords",
base::FeatureList::IsEnabled( base::FeatureList::IsEnabled(
password_manager::features::kPasswordImport)); password_manager::features::kPasswordImport));
......
...@@ -3,6 +3,20 @@ ...@@ -3,6 +3,20 @@
// found in the LICENSE file. // found in the LICENSE file.
cr.define('settings_people_page', function() { cr.define('settings_people_page', function() {
/** @implements {settings.PeopleBrowserProxy} */
class TestPeopleBrowserProxy extends TestBrowserProxy {
constructor() {
super([
'openURL',
]);
}
/** @override */
openURL(url) {
this.methodCalled('openURL', url);
}
}
suite('ProfileInfoTests', function() { suite('ProfileInfoTests', function() {
/** @type {SettingsPeoplePageElement} */ /** @type {SettingsPeoplePageElement} */
let peoplePage = null; let peoplePage = null;
...@@ -501,4 +515,66 @@ cr.define('settings_people_page', function() { ...@@ -501,4 +515,66 @@ cr.define('settings_people_page', function() {
assertEquals(settings.getCurrentRoute(), settings.routes.SYNC); assertEquals(settings.getCurrentRoute(), settings.routes.SYNC);
}); });
}); });
suite('PasswordsUITest', function() {
/** @type {SettingsPeoplePageElement} */
let peoplePage = null;
/** @type {settings.PeopleBrowserProxy} */
let browserProxy = null;
suiteSetup(function() {
// Forces navigation to Google Password Manager to be off by default.
loadTimeData.overrideValues({
navigateToGooglePasswordManager: false,
});
});
setup(function() {
browserProxy = new TestPeopleBrowserProxy();
settings.PeopleBrowserProxyImpl.instance_ = browserProxy;
PolymerTest.clearBody();
peoplePage = document.createElement('settings-people-page');
// Force enable Autofill Home.
// TODO(jdoerrie): https://crbug.com/854562.
// Remove when removing Autofill Home flag.
peoplePage.autofillHomeEnabled = true;
document.body.appendChild(peoplePage);
Polymer.dom.flush();
});
teardown(function() {
peoplePage.remove();
});
test('Google Password Manager Off', function() {
assertTrue(!!peoplePage.$$('#passwordManagerButton'));
peoplePage.$$('#passwordManagerButton').click();
Polymer.dom.flush();
assertEquals(
settings.getCurrentRoute(), settings.routes.MANAGE_PASSWORDS);
});
test('Google Password Manager On', function() {
// Hardcode this value so that the test is independent of the production
// implementation that might include additional query parameters.
const googlePasswordManagerUrl = 'https://passwords.google.com';
loadTimeData.overrideValues({
navigateToGooglePasswordManager: true,
googlePasswordManagerUrl: googlePasswordManagerUrl,
});
assertTrue(!!peoplePage.$$('#passwordManagerButton'));
peoplePage.$$('#passwordManagerButton').click();
Polymer.dom.flush();
return browserProxy.whenCalled('openURL').then(url => {
assertEquals(googlePasswordManagerUrl, url);
});
});
});
}); });
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