Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 11361142: Fill passwords if password manager is disabled. (Closed)

Created:
5 years ago by Timo Reimann
Modified:
5 years ago
Reviewers:
Ilya Sherman
CC:
chromium-reviews
Visibility:
Public.

Description

Fill passwords if password manager is disabled. As of now, filling passwords is directly dependent on the manager being enabled in the settings. However, the password manager is supposed to fill previously stored passwords even when disabled. Changes introduced by the patch are as following: - Remove IsFillingEnabled() and update code relying on method accordingly. - Add test to ensure that password saving depends on manager enabled preference. - Add test to ensure that passwords are filled on a disabled password manager. - Add myself to AUTHORS file. Contributed by Timo Reimann <ttr314@googlemail.com>; R=isherman@chromium.org BUG=158296 Review URL: https://chromiumcodereview.appspot.com/11361142/ Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=166878

Patch Set 1 #

Patch Set 2 : Fix lint warnings. #

Total comments: 4

Patch Set 3 : Make unit tests look consistent; add/extend in-code documentation. #

Messages

Total messages: 8 (0 generated)
Timo Reimann
Hi, here's the CL for http://code.google.com/p/chromium/issues/detail?id=158296 to review. Let me know if I missed anything. ...
5 years ago (2012-11-07 02:56:37 UTC) #1
Ilya Sherman
Thanks, generally looks good -- just a couple of small nits below :) Can you ...
5 years ago (2012-11-08 06:11:26 UTC) #2
Timo Reimann
https://chromiumcodereview.appspot.com/11361142/diff/4002/chrome/browser/password_manager/password_manager.h File chrome/browser/password_manager/password_manager.h (right): https://chromiumcodereview.appspot.com/11361142/diff/4002/chrome/browser/password_manager/password_manager.h#newcode119 chrome/browser/password_manager/password_manager.h:119: // want to save passwords but continue to fill ...
5 years ago (2012-11-08 13:36:06 UTC) #3
Timo Reimann
Updated patch set is here for re-reviewal. I also added short test descriptions to the ...
5 years ago (2012-11-08 13:44:30 UTC) #4
Timo Reimann
I also confirm that I have successfully signed the Individual Contributor License Agreement.
5 years ago (2012-11-08 13:45:12 UTC) #5
Ilya Sherman
LGTM -- thanks for contributing this patch!
5 years ago (2012-11-09 08:05:13 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ttr314@googlemail.com/11361142/2002
5 years ago (2012-11-09 08:05:31 UTC) #7
commit-bot: I haz the power
5 years ago (2012-11-09 09:57:24 UTC) #8
Change committed as 166878

Powered by Google App Engine
This is Rietveld efc10ee0f