[IntentHandling] Clear intent picker bubble if try to show bubble again.

This CL updates the intent picker bubble view code to close and clear
the existing intent picker bubble if the code trying to show another
intent picker bubble. During testing, I found out that showing a new
intent picker bubble when there is already a bubble showing will crash
the browser. And there are suspected crash report that possibly relate
to this.

BUG=1148517, 853604

Change-Id: Ibf720342acf73602608278ca48f71435a9e61e24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2539808
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828639}

chrome/browser/ui/views/intent_picker_bubble_view.cc

@@ -142,11 +142,8 @@ views::Widget* IntentPickerBubbleView::ShowBubble(
     const base::Optional<url::Origin>& initiating_origin,
     IntentPickerResponse intent_picker_cb) {
   if (intent_picker_bubble_) {
-    intent_picker_bubble_->Initialize();
-    views::Widget* widget =
-        views::BubbleDialogDelegateView::CreateBubble(intent_picker_bubble_);
-    widget->Show();
-    return widget;
+    intent_picker_bubble_->CloseBubble();
+    intent_picker_bubble_ = nullptr;
   }
   intent_picker_bubble_ = new IntentPickerBubbleView(
       anchor_view, icon_view, icon_type, std::move(app_info),

chrome/browser/ui/views/intent_picker_bubble_view.h

@@ -126,6 +126,8 @@ class IntentPickerBubbleView : public LocationBarBubbleDelegateView {
                            NotLinkDoesNotShowBubble);
   FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewBrowserTestChromeOS,
                            DismissBubble);
+  FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewBrowserTestChromeOS,
+                           ShowBubbleTwice);
 
   static std::unique_ptr<IntentPickerBubbleView> CreateBubbleViewForTesting(
       views::View* anchor_view,

chrome/browser/ui/views/intent_picker_bubble_view_browsertest_chromeos.cc

@@ -202,11 +202,38 @@ class IntentPickerBubbleViewBrowserTestChromeOS : public InProcessBrowserTest {
     EXPECT_TRUE(intent_picker_bubble()->GetVisible());
   }
 
+  // Dummy method to be called upon bubble closing.
+  void OnBubbleClosed(const std::string& selected_app_package,
+                      apps::PickerEntryType entry_type,
+                      apps::IntentPickerCloseReason close_reason,
+                      bool should_persist) {
+    bubble_closed_ = true;
+  }
+
+  void ShowBubbleForTesting() {
+    std::vector<apps::IntentPickerAppInfo> app_info;
+    app_info.emplace_back(apps::PickerEntryType::kArc, gfx::Image(),
+                          "package_1", "dank app 1");
+    app_info.emplace_back(apps::PickerEntryType::kArc, gfx::Image(),
+                          "package_2", "dank_app_2");
+
+    browser()->window()->ShowIntentPickerBubble(
+        std::move(app_info), /*show_stay_in_chrome=*/true,
+        /*show_remember_selection=*/true, PageActionIconType::kIntentPicker,
+        base::nullopt,
+        base::BindOnce(
+            &IntentPickerBubbleViewBrowserTestChromeOS::OnBubbleClosed,
+            base::Unretained(this)));
+  }
+
+  bool bubble_closed() { return bubble_closed_; }
+
  private:
   apps::AppServiceProxy* app_service_proxy_ = nullptr;
   std::unique_ptr<arc::FakeIntentHelperInstance> intent_helper_instance_;
   std::unique_ptr<arc::FakeAppInstance> app_instance_;
   FakeIconLoader icon_loader_;
+  bool bubble_closed_ = false;
 };
 
 // Test that the intent picker bubble will pop out for ARC apps.
@@ -411,3 +438,18 @@ IN_PROC_BROWSER_TEST_F(IntentPickerBubbleViewBrowserTestChromeOS,
   EXPECT_EQ(app_name, launched_arc_apps()[0].activity->package_name);
   EXPECT_EQ(test_url.spec(), launched_arc_apps()[0].intent->data);
 }
+
+// Test that show intent picker bubble twice without closing doesn't
+// crash the browser.
+IN_PROC_BROWSER_TEST_F(IntentPickerBubbleViewBrowserTestChromeOS,
+                       ShowBubbleTwice) {
+  ShowBubbleForTesting();
+  ASSERT_TRUE(intent_picker_bubble());
+  EXPECT_TRUE(intent_picker_bubble()->GetVisible());
+  EXPECT_EQ(2U, intent_picker_bubble()->GetScrollViewSize());
+  ShowBubbleForTesting();
+  ASSERT_TRUE(bubble_closed());
+  ASSERT_TRUE(intent_picker_bubble());
+  EXPECT_TRUE(intent_picker_bubble()->GetVisible());
+  EXPECT_EQ(2U, intent_picker_bubble()->GetScrollViewSize());
+}