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

chrome/browser/ui/passwords/manage_passwords_ui_controller.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_UI_CONTROLLER_H_
 #define CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_UI_CONTROLLER_H_
 
 #include <vector>
 
 #include "base/macros.h"
@@ skipping 14 matching lines @@
 struct CredentialInfo;
 struct InteractionsStats;
 class PasswordFormManager;
 }
 
 class AccountChooserPrompt;
 class AutoSigninFirstRunPrompt;
 class ManagePasswordsIconView;
 class PasswordDialogController;
 class PasswordDialogControllerImpl;
+class PasswordsModelDelegateProxy;
 
 // Per-tab class to control the Omnibox password icon and bubble.
 class ManagePasswordsUIController
     : public content::WebContentsObserver,
       public content::WebContentsUserData<ManagePasswordsUIController>,
       public password_manager::PasswordStore::Observer,
       public PasswordsModelDelegate,
       public PasswordsClientUIDelegate {
  public:
   ~ManagePasswordsUIController() override;
@@ skipping 27 matching lines @@
       const password_manager::PasswordStoreChangeList& changes) override;
 
   // Set the state of the Omnibox icon, and possibly show the associated bubble
   // without user interaction.
   virtual void UpdateIconAndBubbleState(ManagePasswordsIconView* icon);
 
   bool IsAutomaticallyOpeningBubble() const {
     return bubble_status_ == SHOULD_POP_UP;
   }
 
+  PasswordsModelDelegateProxy* GetModelDelegateProxy() const {
+    return bubble_delegate_proxy_.get();
+  }
+
   // PasswordsModelDelegate:
+  content::WebContents* GetWebContents() const override;
   const GURL& GetOrigin() const override;
   password_manager::ui::State GetState() const override;
   const autofill::PasswordForm& GetPendingPassword() const override;
   bool IsPasswordOverridden() const override;
   const std::vector<const autofill::PasswordForm*>& GetCurrentForms()
       const override;
   const std::vector<const autofill::PasswordForm*>& GetFederatedForms()
       const override;
   password_manager::InteractionsStats* GetCurrentInteractionStats() const
       override;
   void OnBubbleShown() override;
   void OnBubbleHidden() override;
   void OnNoInteractionOnUpdate() override;
   void OnNopeUpdateClicked() override;
   void NeverSavePassword() override;
   void SavePassword() override;
   void UpdatePassword(const autofill::PasswordForm& password_form) override;
   void ChooseCredential(
-      autofill::PasswordForm form,
+      const autofill::PasswordForm& form,
       password_manager::CredentialType credential_type) override;
   void NavigateToExternalPasswordManager() override;
   void NavigateToSmartLockHelpPage() override;
   void NavigateToPasswordManagerSettingsPage() override;
   void NavigateToChromeSignIn() override;
   void OnDialogHidden() override;
 
  protected:
   explicit ManagePasswordsUIController(
       content::WebContents* web_contents);
@@ skipping 51 matching lines @@
 
   // content::WebContentsObserver:
   void WebContentsDestroyed() override;
 
   // The wrapper around current state and data.
   ManagePasswordsState passwords_data_;
 
   // The controller for the blocking dialogs.
   std::unique_ptr<PasswordDialogControllerImpl> dialog_controller_;
 
+  // A proxy for the bubble. The bubbles of different types can pop up

[sky, 2016/08/03 19:42:00]

Forgive me if I am misunderstanding the problem, but it seems like you are
working around the problem. If you are going to show a bubble and there is one
already open, why don't you close the existing one first and ignore anything
from it?

[vasilii, 2016/08/04 09:29:13]

I do close the existing bubble and ignore anything from it. Though it's done in
the platform specific code, not here. Let's say I introduce a method
CloseExistingBubble here. Still on Windows it will be destroyed asynchronously
and I'll get OnBubbleHidden later when the new bubble is open.

There is another benefit of the current approach vs. bool ignore_close_.
ManagePasswordsBubbleModel extends WebContentsObserver for no reason. It just
needs web_contents(). Thus, it makes sense to pass some pointer to it in the
constructor. It can't be WebContents because it can be destroyed. It shouldn't
be ManagePasswordsUIController* either. If I close the tab by Ctrl-W then the
bubble outlives the controller. The bubble on Windows is  destroyed
asynchronously. The bubble on Mac is destroyed after animation finishes. Thus,
there is a good chance of use after free.

+  // unpredictably superseding each other. However, closing the bubble may
+  // affect the state of ManagePasswordsUIController internally. This is
+  // undesired if ManagePasswordsUIController is in the process of opening a new
+  // bubble. The situation is worse on Windows where the bubble is destroyed
+  // asynchronously. Thus, OnBubbleHidden() can be called after the new one is
+  // shown. By that time the internal state of ManagePasswordsUIController has
+  // nothing to do with the old bubble.
+  std::unique_ptr<PasswordsModelDelegateProxy> bubble_delegate_proxy_;
+
   BubbleStatus bubble_status_;
 
   DISALLOW_COPY_AND_ASSIGN(ManagePasswordsUIController);
 };
 
 #endif  // CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_UI_CONTROLLER_H_