Fix crash on a page navigation when Update Password bubble is active.

Fix crash on a page navigation when Update Password bubble is active.

BUG=550934
Committed: https://crrev.com/eeee02d486effbf580cfaa30bff7f5859afe1674
Cr-Commit-Position: refs/heads/master@{#357873}

[dvadym, 2015-11-04 15:54:01]

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

[dvadym, 2015-11-04 15:54:02]

Hi Vasilii,

 Could you please review this CL?

Regards,
Vadym

[vasilii, 2015-11-04 16:12:52]

https://codereview.chromium.org/1416633010/diff/20001/chrome/browser/ui/passw...
File chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
(right):

https://codereview.chromium.org/1416633010/diff/20001/chrome/browser/ui/passw...
chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc:586:
content::FrameNavigateParams());
Check that the state is 'inactive'.

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:882:
DCHECK(!pending_credentials_.new_password_element.empty());
How is it related to the crash?

[dvadym, 2015-11-04 16:18:03]

Description was changed from

==========
Fix crash on a page navigation when Update Password bubble is active.

BUG=550934
==========

to

==========
Fix crash on a page navigation when Update Password bubble is active.

Since changes in this CL we can also fix todo from 549525.

BUG=550934,549525
==========

[dvadym, 2015-11-04 16:20:23]

Thanks Vasilii! I've addressed your comments. PTAL.

https://codereview.chromium.org/1416633010/diff/20001/chrome/browser/ui/passw...
File chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
(right):

https://codereview.chromium.org/1416633010/diff/20001/chrome/browser/ui/passw...
chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc:586:
content::FrameNavigateParams());
On 2015/11/04 16:12:52, vasilii wrote:
> Check that the state is 'inactive'.

Done.

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:882:
DCHECK(!pending_credentials_.new_password_element.empty());
On 2015/11/04 16:12:52, vasilii wrote:
> How is it related to the crash?

Since changes in this CL we can also fix todo from 549525.

[vasilii, 2015-11-04 16:24:36]

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:882:
DCHECK(!pending_credentials_.new_password_element.empty());
On 2015/11/04 16:20:23, dvadym wrote:
> On 2015/11/04 16:12:52, vasilii wrote:
> > How is it related to the crash?
> 
> Since changes in this CL we can also fix todo from 549525.

We can. But in a separate CL only ;-)

[dvadym, 2015-11-04 16:44:15]

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/1416633010/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:882:
DCHECK(!pending_credentials_.new_password_element.empty());
On 2015/11/04 16:24:36, vasilii wrote:
> On 2015/11/04 16:20:23, dvadym wrote:
> > On 2015/11/04 16:12:52, vasilii wrote:
> > > How is it related to the crash?
> > 
> > Since changes in this CL we can also fix todo from 549525.
> 
> We can. But in a separate CL only ;-)

Changes in manage_passwords_ui_controller.cc is already a fix. I've just removed
TODO and added DCHECK. So here we have a fix of that bug for free.

[dvadym, 2015-11-04 17:35:20]

Description was changed from

==========
Fix crash on a page navigation when Update Password bubble is active.

Since changes in this CL we can also fix todo from 549525.

BUG=550934,549525
==========

to

==========
Fix crash on a page navigation when Update Password bubble is active.

BUG=550934
==========

[vasilii, 2015-11-04 17:35:42]

lgtm

[dvadym, 2015-11-04 19:22:43]

The CQ bit was checked by dvadym@chromium.org

[commit-bot: I haz the power, 2015-11-04 19:24:25]

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

[commit-bot: I haz the power, 2015-11-04 19:37:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-04 19:39:58]

Patchset 4 (id:??) landed as
https://crrev.com/eeee02d486effbf580cfaa30bff7f5859afe1674
Cr-Commit-Position: refs/heads/master@{#357873}