Added async reveal to the Tab Search bubble

This change updates TabSearchBubbleView such that it is revealed
asynchronously after its WebContents has finished loading.

This helps in eliminating stuttering that currently occurs due to the
bubble being dynamically resized as the WebContents is loaded in.

Removed call to WasHidden() which causes issues with WebContents
resizing and is not needed for the TabSearch usecase.

Bug: 1099917
Change-Id: I5b5999b00535647fbccddb2b8c88ee1070030347
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264614
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#787869}

chrome/browser/ui/views/tab_search/tab_search_bubble_view.cc

@@ -12,6 +12,12 @@
 #include "chrome/common/webui_url_constants.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/layout/fill_layout.h"
+#include "ui/views/widget/widget.h"
+
+#if defined(USE_AURA)
+#include "ui/aura/window.h"
+#include "ui/wm/public/activation_client.h"
+#endif
 
 namespace {
 
@@ -29,36 +35,96 @@ class TabSearchWebView : public views::WebView {
 
   ~TabSearchWebView() override = default;
 
-  // WebView:
+  // views::WebView:
   void PreferredSizeChanged() override {
-    View::PreferredSizeChanged();
+    WebView::PreferredSizeChanged();
     parent_->OnWebViewSizeChanged();
   }
 
+  void OnWebContentsAttached() override { SetVisible(false); }
+
+  void ResizeDueToAutoResize(content::WebContents* web_contents,
+                             const gfx::Size& new_size) override {
+    // Don't actually do anything with this information until we have been
+    // shown. Size changes will not be honored by lower layers while we are
+    // hidden.
+    if (!GetVisible()) {
+      pending_preferred_size_ = new_size;
+      return;
+    }
+    WebView::ResizeDueToAutoResize(web_contents, new_size);
+  }
+
+  void DocumentOnLoadCompletedInMainFrame() override {
+    GetWidget()->Show();
+    GetWebContents()->Focus();
+  }
+
+  void DidStopLoading() override {
+    if (GetVisible())
+      return;
+
+    SetVisible(true);
+    ResizeDueToAutoResize(web_contents(), pending_preferred_size_);
+  }
+
  private:
   TabSearchBubbleView* parent_;
+
+  // What we should set the preferred width to once TabSearch has loaded.
+  gfx::Size pending_preferred_size_;
 };
 
 }  // namespace
 
+#if defined(USE_AURA)
+class TabSearchBubbleView::TabSearchWindowObserverAura
+    : public wm::ActivationChangeObserver {
+ public:
+  explicit TabSearchWindowObserverAura(TabSearchBubbleView* bubble)
+      : bubble_(bubble) {
+    gfx::NativeView native_view = bubble_->GetWidget()->GetNativeView();
+    // This is removed in the destructor called by
+    // TabSearchBubbleView::OnWidgetDestroying(), which is guaranteed to be
+    // called before the Widget goes away.  It's not safe to use a
+    // ScopedObserver for this, since the activation client may be deleted
+    // without a call back to this class.
+    wm::GetActivationClient(native_view->GetRootWindow())->AddObserver(this);
+  }
+
+  ~TabSearchWindowObserverAura() override {
+    auto* activation_client = wm::GetActivationClient(
+        bubble_->GetWidget()->GetNativeWindow()->GetRootWindow());
+    DCHECK(activation_client);
+    activation_client->RemoveObserver(this);
+  }
+
+  // wm::ActivationChangeObserver:
+  void OnWindowActivated(wm::ActivationChangeObserver::ActivationReason reason,
+                         aura::Window* gained_active,
+                         aura::Window* lost_active) override {
+    // Close on anchor window activation (i.e. user clicked the browser window).
+    // DesktopNativeWidgetAura does not trigger the expected browser widget
+    // [de]activation events when activating widgets in its own root window.
+    // This additional check handles those cases. See https://crbug.com/320889 .
+    views::Widget* anchor_widget = bubble_->anchor_widget();
+    if (anchor_widget && gained_active == anchor_widget->GetNativeWindow()) {
+      bubble_->GetWidget()->CloseWithReason(
+          views::Widget::ClosedReason::kLostFocus);
+    }
+  }
+
+ private:
+  TabSearchBubbleView* bubble_;
+};
+#endif
+
 void TabSearchBubbleView::CreateTabSearchBubble(Browser* browser) {
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
   DCHECK(browser_view);
-  auto delegate = base::WrapUnique(
-      new TabSearchBubbleView(browser, browser_view->toolbar()));
-  BubbleDialogDelegateView::CreateBubble(delegate.release())->Show();
-}
-
-gfx::Size TabSearchBubbleView::CalculatePreferredSize() const {
-  // Constrain the size to popup min/max.
-  gfx::Size preferred_size = views::View::CalculatePreferredSize();
-  preferred_size.SetToMax(kMinSize);
-  preferred_size.SetToMin(kMaxSize);
-  return preferred_size;
-}
-
-void TabSearchBubbleView::OnWebViewSizeChanged() {
-  SizeToContents();
+  auto delegate =
+      std::make_unique<TabSearchBubbleView>(browser, browser_view->toolbar());
+  BubbleDialogDelegateView::CreateBubble(delegate.release());
 }
 
 TabSearchBubbleView::TabSearchBubbleView(Browser* browser,
@@ -66,17 +132,53 @@ TabSearchBubbleView::TabSearchBubbleView(Browser* browser,
     : BubbleDialogDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
       web_view_(AddChildView(
           std::make_unique<TabSearchWebView>(browser->profile(), this))) {
+  observed_anchor_widget_.Add(anchor_view->GetWidget());
+
+  set_close_on_deactivate(false);
+
   SetButtons(ui::DIALOG_BUTTON_NONE);
   set_margins(gfx::Insets());
 
   SetLayoutManager(std::make_unique<views::FillLayout>());
   web_view_->EnableSizingFromWebContents(kMinSize, kMaxSize);
   web_view_->LoadInitialURL(GURL(chrome::kChromeUITabSearchURL));
+}
+
+TabSearchBubbleView::~TabSearchBubbleView() = default;
+
+gfx::Size TabSearchBubbleView::CalculatePreferredSize() const {
+  // Constrain the size to popup min/max.
+  gfx::Size preferred_size = views::View::CalculatePreferredSize();
+  preferred_size.SetToMax(kMinSize);
+  preferred_size.SetToMin(kMaxSize);
+  return preferred_size;
+}
+
+void TabSearchBubbleView::AddedToWidget() {
+  BubbleDialogDelegateView::AddedToWidget();
+  observed_bubble_widget_.Add(GetWidget());
+#if defined(USE_AURA)
+  // |window_observer_| deals with activation issues relevant to Aura platforms.
+  // This special case handling is not needed on Mac.
+  window_observer_ = std::make_unique<TabSearchWindowObserverAura>(this);
+#endif
+}
 
-  // TODO(crbug.com/1010589) WebContents are initially assumed to be visible by
-  // default unless explicitly hidden. The WebContents need to be set to hidden
-  // so that the visibility state of the document in JavaScript is correctly
-  // initially set to 'hidden', and the 'visibilitychange' events correctly get
-  // fired.
-  web_view_->GetWebContents()->WasHidden();
+void TabSearchBubbleView::OnWidgetActivationChanged(views::Widget* widget,
+                                                    bool active) {
+  // The widget is shown asynchronously and may take a long time to appear, so
+  // only close if it's actually been shown.
+  if (GetWidget()->IsVisible() && widget == anchor_widget() && active)
+    GetWidget()->CloseWithReason(views::Widget::ClosedReason::kLostFocus);
+}
+
+void TabSearchBubbleView::OnWidgetDestroying(views::Widget* widget) {
+#if defined(USE_AURA)
+  if (widget == GetWidget())
+    window_observer_.reset();
+#endif
+}
+
+void TabSearchBubbleView::OnWebViewSizeChanged() {
+  SizeToContents();
 }

chrome/browser/ui/views/tab_search/tab_search_bubble_view.h

@@ -5,30 +5,57 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_TAB_SEARCH_TAB_SEARCH_BUBBLE_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_TAB_SEARCH_TAB_SEARCH_BUBBLE_VIEW_H_
 
+#include "base/scoped_observer.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
 
+namespace views {
+class Widget;
+class WidgetObserver;
+}  // namespace views
+
 class Browser;
 
-// TODO(tluk): Only show the bubble once web contents are available to prevent
-// akward resizing when web content finally loads in.
-class TabSearchBubbleView : public views::BubbleDialogDelegateView {
+class TabSearchBubbleView : public views::BubbleDialogDelegateView,
+                            public views::WidgetObserver {
  public:
+  // TODO(tluk): Since the Bubble is shown asynchronously, we shouldn't call
+  // this if the Widget is hidden and yet to be revealed.
   static void CreateTabSearchBubble(Browser* browser);
 
-  ~TabSearchBubbleView() override = default;
+  TabSearchBubbleView(Browser* browser, views::View* anchor_view);
+  ~TabSearchBubbleView() override;
 
   // views::BubbleDialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;
+  void AddedToWidget() override;
+
+  // views::WidgetObserver:
+  void OnWidgetActivationChanged(views::Widget* widget, bool active) override;
+  void OnWidgetDestroying(views::Widget* widget) override;
 
   void OnWebViewSizeChanged();
 
  private:
-  TabSearchBubbleView(Browser* browser, views::View* anchor_view);
+#if defined(USE_AURA)
+  // TabSearchWindowObserverAura deals with issues in bubble deactivation on
+  // Aura platforms. See comments in OnWindowActivated().
+  // These issues are not present on Mac.
+  class TabSearchWindowObserverAura;
+
+  // |window_observer_| is a helper that hooks into the TabSearchBubbleView's
+  // widget lifecycle events.
+  std::unique_ptr<TabSearchWindowObserverAura> window_observer_;
+#endif
 
   views::WebView* web_view_;
 
+  ScopedObserver<views::Widget, views::WidgetObserver> observed_anchor_widget_{
+      this};
+  ScopedObserver<views::Widget, views::WidgetObserver> observed_bubble_widget_{
+      this};
+
   DISALLOW_COPY_AND_ASSIGN(TabSearchBubbleView);
 };