Adding a ScrollView for IntentPickerBubbleView

chrome/browser/ui/views/intent_picker_bubble_view.h

Index: chrome/browser/ui/views/intent_picker_bubble_view.h
diff --git a/chrome/browser/ui/views/intent_picker_bubble_view.h b/chrome/browser/ui/views/intent_picker_bubble_view.h
index ae516f2db1caeb38cbbdf44a6de3a17e6637f78f..14c4ce86b84a4bbb27ae7d39d7a0d230f1dbbe5d 100644
--- a/chrome/browser/ui/views/intent_picker_bubble_view.h
+++ b/chrome/browser/ui/views/intent_picker_bubble_view.h
@@ -21,7 +21,6 @@ class NavigationHandle;
 }  // namespace content
 
 namespace views {
-class EventMonitor;
 class Label;
 class LabelButton;
 class View;
@@ -63,16 +62,21 @@ class IntentPickerBubbleView : public views::BubbleDialogDelegateView,
   static void ShowBubble(content::NavigationHandle* handle,
                          const std::vector<NameAndIcon>& app_info,
                          const ThrottleCallback& throttle_cb);
+  ~IntentPickerBubbleView() override;
 
  protected:
   // views::BubbleDialogDelegateView overrides:
   void Init() override;
 
  private:
+  friend class IntentPickerBubbleViewTest;

[Yusuke Sato, 2016/07/19 10:23:58]

Another way to allow the test to access internals of this class is to add a
public function with _for_testing() or ForTesting() suffix.

 public:
  const std::vector<views::LabelButton*>& app_label_ptr_for_testing() const;

  // or
  size_t GetNumAppLablesForTesting() const;

This allows you to control what test files can see. If you call such a method in
non-test .cc file, trybot fails.

[djacobo_, 2016/07/20 21:18:30]

Done.

+  FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, NullIcons);

[Yusuke Sato, 2016/07/20 08:05:54]

Or, if you go with friend, can you remove line 73-75? Adding one
FRIEND_TEST_ALL_PREFIXES per TEST_F seems too noisy to me.

[djacobo_, 2016/07/20 21:18:30]

Done.

+  FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, NonNullIcons);
+  FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, LabelsPtrVectorSize);
   IntentPickerBubbleView(const std::vector<NameAndIcon>& app_info,
                          ThrottleCallback throttle_cb,
-                         content::WebContents* web_contents);
-  ~IntentPickerBubbleView() override;
+                         content::WebContents* web_contents,
+                         bool was_callback_run);
 
   // views::BubbleDialogDelegateView overrides:
   void OnWidgetDestroying(views::Widget* widget) override;
@@ -101,6 +105,7 @@ class IntentPickerBubbleView : public views::BubbleDialogDelegateView,
   views::LabelButton* just_once_button_;
 
   std::vector<NameAndIcon> app_info_;
+  std::vector<views::LabelButton*> app_label_ptr_;

[Yusuke Sato, 2016/07/19 10:23:58]

Is this really necessary? I'm not familiar with Views at all but ui/views/view.h
has set_id() and GetViewById().

Holding non-weak and non-owning pointers looks a bit scary to me.

[djacobo_, 2016/07/20 21:18:30]

Removed, opting for the GetViewByID() method you suggested, thanks!

 
   DISALLOW_COPY_AND_ASSIGN(IntentPickerBubbleView);
 };