Commit 996b639f authored by Ioana Pandele's avatar Ioana Pandele Committed by Commit Bot

[ReEPwdSave]Don't show toggle to re-enable saving if the password manager is disabled

If the saving of passwords is generally disabled from settings, or if
the save passwords prompt can't be shown, the toggle should not be
shown either.

Bug: 1044930
Change-Id: I017ed562cfc1dc10eddf31306d0e7730cd5206a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2149332
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760536}
parent 9a985270
...@@ -236,7 +236,7 @@ ChromePasswordManagerClient::ChromePasswordManagerClient( ...@@ -236,7 +236,7 @@ ChromePasswordManagerClient::ChromePasswordManagerClient(
&ContentPasswordManagerDriverFactory::RequestSendLoggingAvailability, &ContentPasswordManagerDriverFactory::RequestSendLoggingAvailability,
base::Unretained(driver_factory_))); base::Unretained(driver_factory_)));
saving_and_filling_passwords_enabled_.Init( saving_passwords_enabled_.Init(
password_manager::prefs::kCredentialsEnableService, GetPrefs()); password_manager::prefs::kCredentialsEnableService, GetPrefs());
static base::NoDestructor<password_manager::StoreMetricsReporter> reporter( static base::NoDestructor<password_manager::StoreMetricsReporter> reporter(
this, GetSyncService(profile_), GetIdentityManager(), GetPrefs()); this, GetSyncService(profile_), GetIdentityManager(), GetPrefs());
...@@ -282,10 +282,7 @@ bool ChromePasswordManagerClient::IsSavingAndFillingEnabled( ...@@ -282,10 +282,7 @@ bool ChromePasswordManagerClient::IsSavingAndFillingEnabled(
// page, and there is no API to access (or dismiss) UI bubbles/infobars. // page, and there is no API to access (or dismiss) UI bubbles/infobars.
return false; return false;
} }
// TODO(melandory): remove saving_and_filling_passwords_enabled_ check from return *saving_passwords_enabled_ && !IsIncognito() && IsFillingEnabled(url);
// here once we decide to switch to new settings behavior for everyone.
return *saving_and_filling_passwords_enabled_ && !IsIncognito() &&
IsFillingEnabled(url);
} }
bool ChromePasswordManagerClient::IsFillingEnabled(const GURL& url) const { bool ChromePasswordManagerClient::IsFillingEnabled(const GURL& url) const {
......
...@@ -369,8 +369,9 @@ class ChromePasswordManagerClient ...@@ -369,8 +369,9 @@ class ChromePasswordManagerClient
base::WeakPtr<PasswordGenerationPopupControllerImpl> popup_controller_; base::WeakPtr<PasswordGenerationPopupControllerImpl> popup_controller_;
// Set to false to disable password saving (will no longer ask if you // Set to false to disable password saving (will no longer ask if you
// want to save passwords and also won't fill the passwords). // want to save passwords). There is no pref for disabling filling at this
BooleanPrefMember saving_and_filling_passwords_enabled_; // point.
BooleanPrefMember saving_passwords_enabled_;
const password_manager::SyncCredentialsFilter credentials_filter_; const password_manager::SyncCredentialsFilter credentials_filter_;
......
...@@ -284,7 +284,7 @@ void PasswordAccessoryControllerImpl::RefreshSuggestionsForField( ...@@ -284,7 +284,7 @@ void PasswordAccessoryControllerImpl::RefreshSuggestionsForField(
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
password_manager::features::kRecoverFromNeverSaveAndroid) && password_manager::features::kRecoverFromNeverSaveAndroid) &&
is_password_field && is_password_field &&
!web_contents_->GetBrowserContext()->IsOffTheRecord()) { password_client_->IsSavingAndFillingEnabled(origin.GetURL())) {
BlacklistedStatus blacklisted_status = BlacklistedStatus blacklisted_status =
credential_cache_->GetCredentialStore(origin).GetBlacklistedStatus(); credential_cache_->GetCredentialStore(origin).GetBlacklistedStatus();
if (blacklisted_status == BlacklistedStatus::kWasBlacklisted || if (blacklisted_status == BlacklistedStatus::kWasBlacklisted ||
......
...@@ -40,6 +40,7 @@ ...@@ -40,6 +40,7 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/codec/png_codec.h" #include "ui/gfx/codec/png_codec.h"
#include "url/gurl.h"
#include "url/origin.h" #include "url/origin.h"
namespace { namespace {
...@@ -106,6 +107,8 @@ class MockPasswordManagerClient ...@@ -106,6 +107,8 @@ class MockPasswordManagerClient
UpdateCacheWithBlacklistedForOrigin, UpdateCacheWithBlacklistedForOrigin,
(const url::Origin&, bool)); (const url::Origin&, bool));
MOCK_METHOD(bool, IsSavingAndFillingEnabled, (const GURL&), (const));
password_manager::PasswordStore* GetProfilePasswordStore() const override { password_manager::PasswordStore* GetProfilePasswordStore() const override {
return mock_password_store_; return mock_password_store_;
} }
...@@ -604,6 +607,8 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfIsBlacklisted) { ...@@ -604,6 +607,8 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfIsBlacklisted) {
cache()->SaveCredentialsAndBlacklistedForOrigin( cache()->SaveCredentialsAndBlacklistedForOrigin(
{}, CredentialCache::IsOriginBlacklisted(true), {}, CredentialCache::IsOriginBlacklisted(true),
url::Origin::Create(GURL(kExampleSite))); url::Origin::Create(GURL(kExampleSite)));
ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
.WillByDefault(Return(true));
AccessorySheetData::Builder data_builder(AccessoryTabType::PASSWORDS, AccessorySheetData::Builder data_builder(AccessoryTabType::PASSWORDS,
passwords_empty_str(kExampleDomain)); passwords_empty_str(kExampleDomain));
data_builder data_builder
...@@ -620,39 +625,27 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfIsBlacklisted) { ...@@ -620,39 +625,27 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfIsBlacklisted) {
} }
TEST_F(PasswordAccessoryControllerTest, TEST_F(PasswordAccessoryControllerTest,
NoSaveToggleIfIsBlacklistedAndIncognito) { NoSaveToggleIfIsBlacklistedAndSavingDisabled) {
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature( scoped_feature_list.InitAndEnableFeature(
password_manager::features::kRecoverFromNeverSaveAndroid); password_manager::features::kRecoverFromNeverSaveAndroid);
auto incognito_web_contents(content::WebContentsTester::CreateTestWebContents( // Simulate saving being disabled (e.g. being in incognito or having password
profile()->GetOffTheRecordProfile(), /*site_instance=*/nullptr)); // saving disabled from settings).
content::WebContents* raw_web_contents = incognito_web_contents.get(); ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
.WillByDefault(Return(false));
// Set the correct WebContents for the test and make sure the right frame
// is focused.
SetContents(std::move(incognito_web_contents));
NavigateAndCommit(GURL(kExampleSite));
FocusWebContentsOnMainFrame();
PasswordAccessoryControllerImpl::CreateForWebContentsForTesting(
raw_web_contents, cache(), mock_manual_filling_controller_.AsWeakPtr(),
password_client());
PasswordAccessoryController* incognito_accessory =
PasswordAccessoryControllerImpl::FromWebContents(raw_web_contents);
cache()->SaveCredentialsAndBlacklistedForOrigin( cache()->SaveCredentialsAndBlacklistedForOrigin(
{}, CredentialCache::IsOriginBlacklisted(true), {}, CredentialCache::IsOriginBlacklisted(true),
url::Origin::Create(GURL(kExampleSite))); url::Origin::Create(GURL(kExampleSite)));
// Check that the accessory data passed to the |ManualFillingController| does
// not contain the save passwords toggle.
AccessorySheetData::Builder data_builder(AccessoryTabType::PASSWORDS, AccessorySheetData::Builder data_builder(AccessoryTabType::PASSWORDS,
passwords_empty_str(kExampleDomain)); passwords_empty_str(kExampleDomain));
data_builder.AppendFooterCommand(manage_passwords_str(), data_builder.AppendFooterCommand(manage_passwords_str(),
autofill::AccessoryAction::MANAGE_PASSWORDS); autofill::AccessoryAction::MANAGE_PASSWORDS);
EXPECT_CALL(mock_manual_filling_controller_, EXPECT_CALL(mock_manual_filling_controller_,
RefreshSuggestions(std::move(data_builder).Build())); RefreshSuggestions(std::move(data_builder).Build()));
incognito_accessory->RefreshSuggestionsForField( controller()->RefreshSuggestionsForField(
FocusedFieldType::kFillablePasswordField, FocusedFieldType::kFillablePasswordField,
/*is_manual_generation_available=*/false); /*is_manual_generation_available=*/false);
} }
...@@ -668,6 +661,8 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfWasBlacklisted) { ...@@ -668,6 +661,8 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfWasBlacklisted) {
cache()->UpdateBlacklistedForOrigin( cache()->UpdateBlacklistedForOrigin(
url::Origin::Create(GURL(kExampleSite)), url::Origin::Create(GURL(kExampleSite)),
CredentialCache::IsOriginBlacklisted(false)); CredentialCache::IsOriginBlacklisted(false));
ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
.WillByDefault(Return(true));
AccessorySheetData::Builder data_builder(AccessoryTabType::PASSWORDS, AccessorySheetData::Builder data_builder(AccessoryTabType::PASSWORDS,
passwords_empty_str(kExampleDomain)); passwords_empty_str(kExampleDomain));
data_builder data_builder
......
...@@ -101,6 +101,8 @@ class PasswordManagerClient { ...@@ -101,6 +101,8 @@ class PasswordManagerClient {
// the presence of SSL errors on a page. |url| describes the URL to fill the // the presence of SSL errors on a page. |url| describes the URL to fill the
// password for. It is not necessary the URL of the current page but can be a // password for. It is not necessary the URL of the current page but can be a
// URL of a proxy or subframe. // URL of a proxy or subframe.
// TODO(crbug.com/1071842): This method's name is misleading as it also
// determines whether saving prompts should be shown.
virtual bool IsFillingEnabled(const GURL& url) const; virtual bool IsFillingEnabled(const GURL& url) const;
// Checks if manual filling fallback is enabled for the page that has |url| // Checks if manual filling fallback is enabled for the page that has |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