Password bubble: Deplatformify ManagePasswordsBubbleView.

chrome/browser/ui/passwords/manage_passwords_bubble_model.h

 // Copyright 2013 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_BUBBLE_MODEL_H_
 #define CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_BUBBLE_MODEL_H_
 
+#include "chrome/browser/ui/passwords/manage_passwords_bubble.h"
 #include "components/autofill/core/common/password_form.h"
+#include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "content/public/browser/web_contents_observer.h"
 
 class ManagePasswordsIconController;
 
 namespace content {
 class WebContents;
 }
 
 // This model provides data for the ManagePasswordsBubble and controls the
 // password management actions.
 class ManagePasswordsBubbleModel : public content::WebContentsObserver {
  public:
   explicit ManagePasswordsBubbleModel(content::WebContents* web_contents);

[vabr (Chromium), 2014/04/10 09:30:24]

Would it make sense to comment on the default values for dismissal and creation
reasons assigned by the constructor?

[Mike West, 2014/04/10 09:53:46]

Done.

   virtual ~ManagePasswordsBubbleModel();
 
   enum ManagePasswordsBubbleState {
     PASSWORD_TO_BE_SAVED,
     MANAGE_PASSWORDS_AFTER_SAVING,
     MANAGE_PASSWORDS,
     NEVER_SAVE_PASSWORDS,
   };
 
   enum PasswordAction { REMOVE_PASSWORD, ADD_PASSWORD };
 
+  // Called by the view code when the bubble is shown.
+  void OnBubbleShown(ManagePasswordsBubble::DisplayReason reason);
+
+  // Called by the view code when the bubble is hidden.
+  void OnBubbleHidden();
+
   // Called by the view code when the "Nope" button in clicked by the user.
   void OnNopeClicked();
 
   // Called by the view code when the "Never for this site." button in clicked
   // by the user.
   void OnNeverForThisSiteClicked();
 
   // Called by the view code when the save button in clicked by the user.
   void OnSaveClicked();
 
+  // Called by the view code when the "Done" button is clicked by the user.
+  void OnDoneClicked();
+
   // Called by the view code when the manage link is clicked by the user.
   void OnManageLinkClicked();
 
   // Called by the view code to delete or add a password form to the
   // PasswordStore.
   void OnPasswordAction(const autofill::PasswordForm& password_form,
                         PasswordAction action);
 
+  // Close the bubble without logging.

[vabr (Chromium), 2014/04/10 09:30:24]

nit: This does not really say more than the function name. Culd you elaborate on
the "logging"? (Is it UMA?)

+  void OnCloseWithoutLogging();
+
   // Called by the view code when the ManagePasswordItemView is destroyed and
   // the user chose to delete the password.
   // TODO(npentrel): Remove this once best_matches_ are newly made on bubble
   // opening.
   void DeleteFromBestMatches(autofill::PasswordForm password_form);
 
   ManagePasswordsBubbleState manage_passwords_bubble_state() {
     return manage_passwords_bubble_state_;
   }
 
   bool WaitingToSavePassword() {
     return manage_passwords_bubble_state() == PASSWORD_TO_BE_SAVED;
   }
 
   bool password_submitted() { return password_submitted_; }
   const base::string16& title() { return title_; }
   const autofill::PasswordForm& pending_credentials() {
     return pending_credentials_;
   }
   const autofill::PasswordFormMap& best_matches() { return best_matches_; }
   const base::string16& manage_link() { return manage_link_; }
 
+  // Gets and sets the reason the bubble was displayed.
+  password_manager::metrics_util::UIDisplayDisposition display_disposition()
+      const {
+    return display_disposition_;
+  }
+  void set_display_disposition(

[vabr (Chromium), 2014/04/10 09:30:24]

I might be mistaken, but all these 4 accessors only used within the class? If
so, then I suggest getting rid of them, or making them private.

[Mike West, 2014/04/10 09:53:46]

They're also used in the tests I haven't finished writing yet.

[vabr (Chromium), 2014/04/10 10:24:44]

But then please mark them as for testing only, and use direct access in the
class code. It will make the code clearer, this way it 1) looks like everybody
is welcome to change these members, and 2) inside the class it's confusing to
see the accessors (is it modifying something outside of the class? is there some
side-effect/processing?).

+      password_manager::metrics_util::UIDisplayDisposition disposition) {
+    display_disposition_ = disposition;
+  }
+
+  // Gets and sets the reason the bubble was dismissed.
+  password_manager::metrics_util::UIDismissalReason dismissal_reason() const {
+    return dismissal_reason_;
+  }
+  void set_dismissal_reason(
+      password_manager::metrics_util::UIDismissalReason reason) {
+    dismissal_reason_ = reason;
+  }
+
  private:
   // content::WebContentsObserver
   virtual void WebContentsDestroyed(
       content::WebContents* web_contents) OVERRIDE;
 
   content::WebContents* web_contents_;
   ManagePasswordsBubbleState manage_passwords_bubble_state_;
   bool password_submitted_;
   base::string16 title_;
   autofill::PasswordForm pending_credentials_;
   autofill::PasswordFormMap best_matches_;
   base::string16 manage_link_;
 
+  password_manager::metrics_util::UIDisplayDisposition display_disposition_;
+  password_manager::metrics_util::UIDismissalReason dismissal_reason_;
+
   DISALLOW_COPY_AND_ASSIGN(ManagePasswordsBubbleModel);
 };
 
 #endif  // CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_BUBBLE_MODEL_H_