Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(507)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 151413008: Move ownership of Password(Generation)Manager to ContentPasswordDriver. (Closed)

Created:
6 years, 10 months ago by blundell
Modified:
6 years, 10 months ago
Reviewers:
Garrett Casto
CC:
chromium-reviews, benquan, tfarina, Dane Wallinga, dyu1, estade+watch_chromium.org, Ilya Sherman, rouslan+autofillwatch_chromium.org, vabr (Chromium), Patrick Dubroy
Visibility:
Public.

Description

Move ownership of Password(Generation)Manager to ContentPasswordDriver. Instead of PasswordManager and PasswordGenerationManager being WebContentsUserData instances, they are changed to be owned by ContentPasswordManager (which is in turn owned by PasswordManagerDelegateImpl, which is a WCUD). Convenience functions are added to PasswordManagerDelegateImpl in order to get Password(Generation)Manager from a WebContents instance. BUG=335099, 335026 TBR=thakis Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248924

Patch Set 1 #

Patch Set 2 : Fix up unittests #

Total comments: 2

Patch Set 3 : Rebase to origin/master #

Patch Set 4 : Nits #

Patch Set 5 : Response to review, fix ChromeOS #

Patch Set 6 : Handle WebContents having no PasswordManagerDelegateImpl attached #

Unified diffs Side-by-side diffs Delta from patch set Stats (+170 lines, -124 lines) Patch
M chrome/browser/chromeos/login/simple_web_view_dialog.cc View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/webui_login_view.cc View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/password_manager/content_password_manager_driver.h View 2 chunks +14 lines, -3 lines 0 comments Download
M chrome/browser/password_manager/content_password_manager_driver.cc View 2 chunks +17 lines, -9 lines 0 comments Download
M chrome/browser/password_manager/password_form_manager_unittest.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/password_manager/password_generation_interactive_uitest.cc View 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/password_manager/password_generation_manager.h View 1 2 3 4 chunks +15 lines, -7 lines 0 comments Download
M chrome/browser/password_manager/password_generation_manager.cc View 1 2 3 4 4 chunks +9 lines, -6 lines 0 comments Download
M chrome/browser/password_manager/password_generation_manager_unittest.cc View 1 5 chunks +4 lines, -16 lines 0 comments Download
M chrome/browser/password_manager/password_manager.h View 3 chunks +3 lines, -12 lines 0 comments Download
M chrome/browser/password_manager/password_manager.cc View 3 chunks +2 lines, -16 lines 0 comments Download
M chrome/browser/password_manager/password_manager_delegate_impl.h View 2 chunks +14 lines, -0 lines 0 comments Download
M chrome/browser/password_manager/password_manager_delegate_impl.cc View 1 2 3 4 5 3 chunks +22 lines, -4 lines 0 comments Download
M chrome/browser/password_manager/password_manager_driver.h View 2 chunks +9 lines, -1 line 0 comments Download
M chrome/browser/password_manager/password_manager_unittest.cc View 1 6 chunks +18 lines, -15 lines 0 comments Download
M chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc View 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 2 chunks +10 lines, -9 lines 0 comments Download
M chrome/browser/ui/gtk/password_generation_bubble_gtk.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/login/login_prompt.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/tab_helpers.cc View 3 chunks +4 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 2 chunks +10 lines, -10 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
blundell
Dependent on https://codereview.chromium.org/152693003/.
6 years, 10 months ago (2014-02-03 20:48:10 UTC) #1
Garrett Casto
lgtm https://codereview.chromium.org/151413008/diff/60001/chrome/browser/password_manager/password_generation_manager.cc File chrome/browser/password_manager/password_generation_manager.cc (right): https://codereview.chromium.org/151413008/diff/60001/chrome/browser/password_manager/password_generation_manager.cc#newcode131 chrome/browser/password_manager/password_generation_manager.cc:131: // TODO(blundell): Should this short-circuit out if web_contents() ...
6 years, 10 months ago (2014-02-04 12:53:35 UTC) #2
blundell
TBR=thakis for changes to //chrome outside of //chrome/browser/password_manager https://codereview.chromium.org/151413008/diff/60001/chrome/browser/password_manager/password_generation_manager.cc File chrome/browser/password_manager/password_generation_manager.cc (right): https://codereview.chromium.org/151413008/diff/60001/chrome/browser/password_manager/password_generation_manager.cc#newcode131 chrome/browser/password_manager/password_generation_manager.cc:131: // ...
6 years, 10 months ago (2014-02-04 13:16:17 UTC) #3
blundell
The CQ bit was checked by blundell@chromium.org
6 years, 10 months ago (2014-02-04 13:16:22 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/151413008/360001
6 years, 10 months ago (2014-02-04 13:16:45 UTC) #5
blundell
The CQ bit was unchecked by blundell@chromium.org
6 years, 10 months ago (2014-02-04 13:38:50 UTC) #6
skia-commit-bot
CQ bit was unchecked on CL. Ignoring.
6 years, 10 months ago (2014-02-04 13:38:52 UTC) #7
blundell
The CQ bit was checked by blundell@chromium.org
6 years, 10 months ago (2014-02-04 13:46:40 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/151413008/600001
6 years, 10 months ago (2014-02-04 13:46:44 UTC) #9
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-04 14:31:20 UTC) #10
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) sync_integration_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&number=223160
6 years, 10 months ago (2014-02-04 14:31:21 UTC) #11
skia-commit-bot
CQ bit was unchecked on CL. Ignoring.
6 years, 10 months ago (2014-02-04 14:31:25 UTC) #12
blundell
The CQ bit was checked by blundell@chromium.org
6 years, 10 months ago (2014-02-04 14:46:03 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/151413008/600001
6 years, 10 months ago (2014-02-04 14:46:39 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-04 16:06:15 UTC) #15
commit-bot: I haz the power
Retried try job too often on ios_dbg_simulator for step(s) url_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_simulator&number=120849
6 years, 10 months ago (2014-02-04 16:06:16 UTC) #16
skia-commit-bot
CQ bit was unchecked on CL. Ignoring.
6 years, 10 months ago (2014-02-04 16:06:25 UTC) #17
blundell
The CQ bit was checked by blundell@chromium.org
6 years, 10 months ago (2014-02-05 06:54:55 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/151413008/600001
6 years, 10 months ago (2014-02-05 06:57:09 UTC) #19
commit-bot: I haz the power
6 years, 10 months ago (2014-02-05 08:38:50 UTC) #20
Message was sent while issue was closed. 
Change committed as 248924

Powered by Google App Engine
This is Rietveld 408576698