New issue
Advanced search Search tips

Issue 632408 link

Starred by 2 users
Status: Fixed
Owner:
vasi...@chromium.org
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Improper handling of two password bubbles

Project Member Reported by vasi...@chromium.org, Jul 28 2016 
Version: 52.0.2743.82 (extension 52.0.2743.48)
OS: All

What steps will reproduce the problem?
(1) Open https://9bf5d6a9ae58c0976f64da8a482db42e19365982.googledrive.com/host/0B4ln_TN4AjcNbzd3RDNQYlJRVG8
(2) Save a password there.
(3) In the developer console type "navigator.credentials.get({password: true})"
(4) Again "navigator.credentials.get({password: true})"

What is the expected output?
One toast after another.

What do you see instead?
Depending on the platform. A DCHECK on Mac. On Linux the bubble is just hidden.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 12 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d3f7345020554fbf2c0ab10d3c5171cbd36687a

commit 2d3f7345020554fbf2c0ab10d3c5171cbd36687a
Author: vasilii <vasilii@chromium.org>
Date: Fri Aug 12 18:28:58 2016

Ignore OnBubbleHidden() event when the password bubble is reopened.

- ManagePasswordsUIController talks to ManagePasswordsBubbleModel though a proxy class. It can be destroyed by the controller so the bubble can't reach the controller with OnBubbleHidden message.
- ManagePasswordsBubbleModel doesn't need to be WebContentsObserver. It should take a pointer to the delegate directly.
- Most unit test can use a new lightweight PasswordsModelDelegateMock. There is no reason to instantiate ManagePasswordsUIControllerMock.

BUG= 632408 

Review-Url: https://codereview.chromium.org/2202373002
Cr-Commit-Position: refs/heads/master@{#411709}

[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/password_manager/credential_manager_browsertest.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/password_manager/password_manager_test_base.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/password_manager/password_manager_test_base.h
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/cocoa/passwords/base_passwords_controller_test.h
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/cocoa/passwords/base_passwords_controller_test.mm
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/cocoa/passwords/passwords_bubble_cocoa.mm
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/cocoa/passwords/passwords_bubble_cocoa_unittest.mm
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/manage_passwords_bubble_model.h
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/manage_passwords_ui_controller_mock.h
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/password_dialog_controller_impl_unittest.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/passwords_model_delegate.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/passwords_model_delegate.h
[add] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/passwords_model_delegate_mock.cc
[add] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/passwords/passwords_model_delegate_mock.h
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
[modify] https://crrev.com/2d3f7345020554fbf2c0ab10d3c5171cbd36687a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_interactive_uitest.cc

Comment 2 by vasi...@chromium.org, Aug 16 2016

Status: Fixed (was: Started)
►

Sign in to add a comment