Refactor ManagePasswordsBubbleModel.

Refactor ManagePasswordsBubbleModel.

Create a subclass responsible for updating the statistics. Using InteractionKeeper it's possible to do it not only on bubble destruction but when the bubble updates its content.

BUG=615825
Committed: https://crrev.com/1a3db4d18d2d8c243da9a7e4b8c30c709dbbf352
Cr-Commit-Position: refs/heads/master@{#397111}

[vasilii, 2016-05-31 17:15:40]

vasilii@chromium.org changed reviewers:
+ vabr@chromium.org

[vasilii, 2016-05-31 17:15:41]

Hi Vaclav,

please review.

[vabr (Chromium), 2016-05-31 18:38:22]

LGTM with some nits and questions.

Cheers,
Vaclav

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
File chrome/browser/ui/passwords/manage_passwords_bubble_model.cc (right):

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_bubble_model.cc:10: #include
<string>
#include <limits>
for std::numeric_limits

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_bubble_model.cc:81:
~InteractionKeeper() = default;
nit: Please separate from the constructor with a blank line.

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_bubble_model.cc:152: profile,
ServiceAccessType::EXPLICIT_ACCESS).get();
Should this be IMPLICIT_ACCESS? In other words, is this a result of some user
action? If not, we should use IMPLICIT_ACCESS to avoid data being silently
written in Incognito in case this code gets ever executed by mistake inside
Incognito.

[vasilii, 2016-06-01 10:21:54]

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
File chrome/browser/ui/passwords/manage_passwords_bubble_model.cc (right):

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_bubble_model.cc:10: #include
<string>
On 2016/05/31 18:38:22, vabr (Chromium) wrote:
> #include <limits>
> for std::numeric_limits

Done.

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_bubble_model.cc:81:
~InteractionKeeper() = default;
On 2016/05/31 18:38:22, vabr (Chromium) wrote:
> nit: Please separate from the constructor with a blank line.

Done.

https://codereview.chromium.org/2025003002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_bubble_model.cc:152: profile,
ServiceAccessType::EXPLICIT_ACCESS).get();
On 2016/05/31 18:38:22, vabr (Chromium) wrote:
> Should this be IMPLICIT_ACCESS? In other words, is this a result of some user
> action? If not, we should use IMPLICIT_ACCESS to avoid data being silently
> written in Incognito in case this code gets ever executed by mistake inside
> Incognito.

Done.

[vasilii, 2016-06-01 10:22:19]

The CQ bit was checked by vasilii@chromium.org

[vasilii, 2016-06-01 10:22:19]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/2025003002/#ps20001
(title: "comments")

[commit-bot: I haz the power, 2016-06-01 10:22:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2025003002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2025003002/20001

[commit-bot: I haz the power, 2016-06-01 12:20:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-01 12:21:33]

Description was changed from

==========
Refactor ManagePasswordsBubbleModel.

Create a subclass responsible for updating the statistics. Using
InteractionKeeper it's possible to do it not only on bubble destruction but when
the bubble updates its content.

BUG=615825
==========

to

==========
Refactor ManagePasswordsBubbleModel.

Create a subclass responsible for updating the statistics. Using
InteractionKeeper it's possible to do it not only on bubble destruction but when
the bubble updates its content.

BUG=615825

Committed: https://crrev.com/1a3db4d18d2d8c243da9a7e4b8c30c709dbbf352
Cr-Commit-Position: refs/heads/master@{#397111}
==========

[commit-bot: I haz the power, 2016-06-01 12:21:34]

Patchset 2 (id:??) landed as
https://crrev.com/1a3db4d18d2d8c243da9a7e4b8c30c709dbbf352
Cr-Commit-Position: refs/heads/master@{#397111}