Reusing Ok/Cancel buttons for intent picker

chrome/browser/ui/views/intent_picker_bubble_view.h

 // Copyright 2016 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_VIEWS_INTENT_PICKER_BUBBLE_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_INTENT_PICKER_BUBBLE_VIEW_H_
 
 #include <memory>
 #include <vector>
 
@@ skipping 14 matching lines @@
 class Label;
 class LabelButton;
 class View;
 class Widget;
 }  // namespace views
 
 namespace ui {
 class Event;
 }  // namespace ui
 
+class IntentPickerMenuButton;
+
 // A bubble that displays a list of aplications (icons and names), after the
 // list we show a pair of buttons which allow the user to remember the selection
 // or not. This class comunicates the user's selection with a callback used by
 // ArcNavigationThrottle.
 //   +--------------------------------+
 //   | Open with                      |
 //   |                                |
 //   | Icon1  Name1                   |
 //   | Icon2  Name2                   |
 //   |  ...                           |
 //   | Icon(N) Name(N)                |
 //   |                                |
 //   |           [JUST ONCE] [ALWAYS] |
 //   +--------------------------------+
 
 class IntentPickerBubbleView : public views::BubbleDialogDelegateView,
                                public views::ButtonListener,
                                public content::WebContentsObserver {
  public:
   using NameAndIcon = arc::ArcNavigationThrottle::NameAndIcon;
   // This callback informs the index of the app selected by the user, along with
   // the reason why the Bubble was closed. The size_t param must have a value in
   // the range [0, app_info.size()-1], except when the CloseReason is ERROR or
   // DIALOG_DEACTIVATED, for these cases we return a dummy value
   // |kAppTagNoneSelected| which won't be used at all and has no significance.
   using ThrottleCallback =

[Evan Stade, 2016/09/14 17:48:00]

I thought we were going to change the name of this?

[djacobo_, 2016/09/14 21:29:35]

We agreed on changing the callback name for the incognito dialog, but I agree we
should do it in here too. let me know what you think, Done.

       base::Callback<void(size_t, arc::ArcNavigationThrottle::CloseReason)>;

[Evan Stade, 2016/09/14 17:48:00]

I'm told we should use int for indexes and not to use size_t as an indication
that the value can't be negative.

[djacobo_, 2016/09/14 21:29:35]

I am not using size_t just to ensure we have something non-negative, the  main
reason is I am using that value directly as an index for a vector on the method
we access via the callback, given the style guide it makes sense to me to keep
this type, but I can agree to change it if required
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
wdyt?

[Evan Stade, 2016/09/14 21:53:08]

okey dokey, size_t it is

[djacobo_, 2016/09/16 01:23:08]

Given our conversation I am using package_name to communicate externally (i.e.
with the Navigation Throttle that initially calls the UI), but internally it
still makes sense to me to keep a simple integer that matches the position in
the ScrollView, wdyt?

 
   ~IntentPickerBubbleView() override;
   static void ShowBubble(content::NavigationHandle* handle,
                          const std::vector<NameAndIcon>& app_info,
                          const ThrottleCallback& throttle_cb);
   static std::unique_ptr<IntentPickerBubbleView> CreateBubbleView(
       const std::vector<NameAndIcon>& app_info,
       const ThrottleCallback& throttle_cb,
       content::WebContents* web_contents);
 
+  // views::DialogDelegate overrides:
+  bool Accept() override;
+  bool Cancel() override;
+  bool Close() override;
+  int GetDefaultDialogButton() const override;
+
  protected:
   // views::BubbleDialogDelegateView overrides:
   void Init() override;
 
+  // views::DialogDelegate overrides:
+  bool IsDialogButtonEnabled(ui::DialogButton button) const override;
+
+  // views::WidgetDelegate overrides:
+  base::string16 GetWindowTitle() const override;
+  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
+
  private:
   friend class IntentPickerBubbleViewTest;
   FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, NullIcons);
   FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, NonNullIcons);
   FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, LabelsPtrVectorSize);
+  FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, VerifyStartingInkDrop);
+  FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, InkDropStateTransition);
+  FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, PressButtonTwice);
   IntentPickerBubbleView(const std::vector<NameAndIcon>& app_info,
                          ThrottleCallback throttle_cb,
                          content::WebContents* web_contents);
 
   // views::BubbleDialogDelegateView overrides:
   void OnWidgetDestroying(views::Widget* widget) override;
-  int GetDialogButtons() const override;
 
   // views::ButtonListener overrides:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // views::View overrides:
   gfx::Size GetPreferredSize() const override;
 
   // content::WebContentsObserver overrides:
   void WebContentsDestroyed() override;
 
-  // Retrieves the LabelButton* contained at position |index| from the internal
-  // ScrollView.
-  views::LabelButton* GetLabelButtonAt(size_t index);
-  void SetLabelButtonBackgroundColor(size_t index, SkColor color);
+  // Retrieves the IntentPickerMenuButton* contained at position |index| from
+  // the internal ScrollView.
+  IntentPickerMenuButton* GetIntentPickerMenuButtonAt(size_t index);
 
-  // Flag set to true iff the callback was Run at some previous step, used to
-  // ensure we only use the callback once.
-  bool was_callback_run_;
+  gfx::ImageSkia GetAppImageForTesting(size_t index);
+  views::InkDropState GetInkDropStateForTesting(size_t);
+  void PressButtonForTesting(size_t index, const ui::Event& event);
 
   // Callback used to respond to ArcNavigationThrottle.
   ThrottleCallback throttle_cb_;
 
   // Keeps a invalid value unless the user explicitly makes a decision.
   size_t selected_app_tag_;
 
-  views::LabelButton* always_button_;
-  views::LabelButton* just_once_button_;
   views::ScrollView* scroll_view_;
 
   std::vector<NameAndIcon> app_info_;
 
   DISALLOW_COPY_AND_ASSIGN(IntentPickerBubbleView);
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_INTENT_PICKER_BUBBLE_VIEW_H_