New issue
Advanced search Search tips

Issue 605927 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Oct 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task

Blocking:
issue 341477



Sign in to add a comment

Separate PasswordManagerHandler::HandlePassword* from PasswordManagerHandler

Project Member Reported by vabr@chromium.org, Apr 22 2016

Issue description

PasswordManagerHandler (PMH) has two methods, HandlePasswordImport and HandlePasswordExport, which are fairly independent of the rest or PMH.

This not only adds to the code bloat in PMH, but also makes its testing harder and more prone to mistakes (PMH now has its own testing subclass in its own unittest).

Instead we should separate the functionality of the two methods in a separate class (PasswordImportExportDialogHandler?), owned by PMH but unaware of PMH. The class could also remove some code duplication between the two methods. 

Furthermore, another new class (PasswordImportExportDialogListener?) should become the dialogue listener instead of PMH.

This will make unit testing through mocking much easier, and improve the code structure by decreasing the implicit dependencies and code bloat.

This is based on https://codereview.chromium.org/1193143003/diff/850001/chrome/browser/ui/webui/options/password_manager_handler_unittest.cc#newcode200.
 

Comment 1 by vabr@chromium.org, Jun 6 2016

Labels: -refactoring Hotlist-Refactoring
Project Member

Comment 2 by sheriffbot@chromium.org, Jun 6 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 3 by vabr@chromium.org, Jun 7 2017

Labels: -Type-Bug -Hotlist-Recharge-Cold Type-Task
Status: Available (was: Untriaged)
Cc: cfroussios@chromium.org
Project Member

Comment 5 by sheriffbot@chromium.org, Oct 9

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: WontFix (was: Untriaged)
PasswordManagerHandler is no more.

Sign in to add a comment