Separate PasswordManagerHandler::HandlePassword* from PasswordManagerHandler |
||||||
Issue descriptionPasswordManagerHandler (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.
,
Jun 6 2017
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
,
Jun 7 2017
,
Oct 9 2017
,
Oct 9
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
,
Oct 10
PasswordManagerHandler is no more. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by vabr@chromium.org
, Jun 6 2016