Commit 0f5a8880 authored by Thomas Lukaszewicz's avatar Thomas Lukaszewicz Committed by Commit Bot

Tab Search: Update Tab Search to use the WebBubbleDialogView

This CL removes the bespoke implementation of a WebUI bubble from
Tab Search and switches to using the WebBubbleDialogView.

Bug: 1099917
Change-Id: I5550761c2a98a2f6af5689fa699346eb3911e776
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462344
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: default avatarYuheng Huang <yuhengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818794}
parent 1083117b
......@@ -1477,7 +1477,6 @@ static_library("ui") {
"webui/tab_search/tab_search_page_handler.h",
"webui/tab_search/tab_search_ui.cc",
"webui/tab_search/tab_search_ui.h",
"webui/tab_search/tab_search_ui_embedder.h",
"webui/theme_handler.cc",
"webui/theme_handler.h",
"webui/theme_source.cc",
......
......@@ -8,78 +8,24 @@
#include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_ui.h"
#include "chrome/common/webui_url_constants.h"
#include "ui/gfx/geometry/rounded_corners_f.h"
#include "ui/views/controls/webview/webview.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/widget/widget.h"
namespace {
// The min / max size available to the TabSearchBubbleView.
// These are arbitrary sizes that match those set by ExtensionPopup.
// TODO(tluk): Determine the correct size constraints for the
// TabSearchBubbleView.
constexpr gfx::Size kMinSize(25, 25);
constexpr gfx::Size kMaxSize(800, 600);
class TabSearchWebView : public views::WebView {
public:
TabSearchWebView(content::BrowserContext* browser_context,
TabSearchBubbleView* parent)
: WebView(browser_context), parent_(parent) {}
~TabSearchWebView() override = default;
// views::WebView:
void PreferredSizeChanged() override {
WebView::PreferredSizeChanged();
parent_->OnWebViewSizeChanged();
}
// content::WebContentsDelegate:
bool HandleContextMenu(content::RenderFrameHost* render_frame_host,
const content::ContextMenuParams& params) override {
// Ignores context menu.
return true;
}
private:
TabSearchBubbleView* parent_;
};
} // namespace
// static.
views::Widget* TabSearchBubbleView::CreateTabSearchBubble(
content::BrowserContext* browser_context,
views::View* anchor_view) {
return BubbleDialogDelegateView::CreateBubble(
std::make_unique<TabSearchBubbleView>(browser_context, anchor_view));
return views::WebBubbleDialogView::CreateWebBubbleDialog<TabSearchUI>(
std::make_unique<TabSearchBubbleView>(browser_context, anchor_view),
GURL(chrome::kChromeUITabSearchURL));
}
TabSearchBubbleView::TabSearchBubbleView(
content::BrowserContext* browser_context,
views::View* anchor_view)
: BubbleDialogDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
web_view_(AddChildView(
std::make_unique<TabSearchWebView>(browser_context, this))) {
SetButtons(ui::DIALOG_BUTTON_NONE);
set_margins(gfx::Insets());
SetLayoutManager(std::make_unique<views::FillLayout>());
// Required for intercepting extension function calls when the page is loaded
// in a bubble (not a full tab, thus tab helpers are not registered
// automatically).
: WebBubbleDialogView(browser_context, anchor_view) {
extensions::ChromeExtensionWebContentsObserver::CreateForWebContents(
web_view_->GetWebContents());
web_view_->EnableSizingFromWebContents(kMinSize, kMaxSize);
web_view_->LoadInitialURL(GURL(chrome::kChromeUITabSearchURL));
TabSearchUI* const tab_search_ui = static_cast<TabSearchUI*>(
web_view_->GetWebContents()->GetWebUI()->GetController());
// Depends on the TabSearchUI object being constructed synchronously when the
// navigation is started in LoadInitialURL().
tab_search_ui->SetEmbedder(this);
web_view()->GetWebContents());
}
TabSearchBubbleView::~TabSearchBubbleView() {
......@@ -89,40 +35,14 @@ TabSearchBubbleView::~TabSearchBubbleView() {
}
}
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();
WebBubbleDialogView::AddedToWidget();
observed_bubble_widget_.Add(GetWidget());
web_view_->holder()->SetCornerRadii(gfx::RoundedCornersF(GetCornerRadius()));
}
void TabSearchBubbleView::ShowBubble() {
DCHECK(GetWidget());
GetWidget()->Show();
web_view_->GetWebContents()->Focus();
timer_ = base::ElapsedTimer();
}
void TabSearchBubbleView::CloseBubble() {
DCHECK(GetWidget());
GetWidget()->CloseWithReason(views::Widget::ClosedReason::kEscKeyPressed);
}
void TabSearchBubbleView::OnWidgetClosing(views::Widget* widget) {
if (widget == GetWidget()) {
TabSearchUI* const tab_search_ui = static_cast<TabSearchUI*>(
web_view_->GetWebContents()->GetWebUI()->GetController());
tab_search_ui->SetEmbedder(nullptr);
void TabSearchBubbleView::OnWidgetVisibilityChanged(views::Widget* widget,
bool visible) {
if (GetWidget() == widget && visible && !timer_.has_value()) {
timer_ = base::ElapsedTimer();
}
}
void TabSearchBubbleView::OnWebViewSizeChanged() {
SizeToContents();
}
......@@ -7,12 +7,10 @@
#include "base/scoped_observer.h"
#include "base/timer/elapsed_timer.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_ui_embedder.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
#include "ui/views/controls/webview/web_bubble_dialog_view.h"
namespace views {
class Widget;
class WebView;
} // namespace views
namespace content {
......@@ -29,45 +27,35 @@ enum class TabSearchOpenAction {
kMaxValue = kTouchGesture,
};
class TabSearchBubbleView : public views::BubbleDialogDelegateView,
public TabSearchUIEmbedder,
class TabSearchBubbleView : public views::WebBubbleDialogView,
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 views::Widget* CreateTabSearchBubble(
content::BrowserContext* browser_context,
views::View* anchor_view);
TabSearchBubbleView(content::BrowserContext* browser_context,
views::View* anchor_view);
TabSearchBubbleView(const TabSearchBubbleView&) = delete;
TabSearchBubbleView& operator=(const TabSearchBubbleView&) = delete;
~TabSearchBubbleView() override;
// views::BubbleDialogDelegateView:
gfx::Size CalculatePreferredSize() const override;
// views::WebBubbleDialogView:
void AddedToWidget() override;
// TabSearchUIEmbedder:
void ShowBubble() override;
void CloseBubble() override;
// views::WidgetObserver:
void OnWidgetClosing(views::Widget* widget) override;
void OnWebViewSizeChanged();
void OnWidgetVisibilityChanged(views::Widget* widget, bool visible) override;
views::WebView* web_view_for_testing() { return web_view_; }
const base::Optional<base::ElapsedTimer>& timer_for_testing() {
return timer_;
}
private:
views::WebView* web_view_;
// Time the Tab Search window has been open.
base::Optional<base::ElapsedTimer> timer_;
ScopedObserver<views::Widget, views::WidgetObserver> observed_bubble_widget_{
this};
DISALLOW_COPY_AND_ASSIGN(TabSearchBubbleView);
};
#endif // CHROME_BROWSER_UI_VIEWS_TAB_SEARCH_TAB_SEARCH_BUBBLE_VIEW_H_
......@@ -13,7 +13,7 @@
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/tabs/tab_search_button.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_ui.h"
#include "chrome/common/webui_url_constants.h"
#include "content/public/test/browser_test.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
......@@ -29,12 +29,13 @@ class TabSearchBubbleBrowserTest : public InProcessBrowserTest {
// InProcessBrowserTest:
void SetUpOnMainThread() override {
auto* anchor = BrowserView::GetBrowserViewForBrowser(browser())->toolbar();
auto* anchor =
BrowserView::GetBrowserViewForBrowser(browser())->GetTabSearchButton();
auto bubble_delegate =
std::make_unique<TabSearchBubbleView>(browser()->profile(), anchor);
bubble_view_ = bubble_delegate.get();
bubble_ = views::BubbleDialogDelegateView::CreateBubble(
bubble_delegate.release());
bubble_ = views::WebBubbleDialogView::CreateWebBubbleDialog<TabSearchUI>(
std::move(bubble_delegate), GURL(chrome::kChromeUITabSearchURL));
}
views::Widget* bubble() { return bubble_; }
......@@ -46,53 +47,17 @@ class TabSearchBubbleBrowserTest : public InProcessBrowserTest {
base::test::ScopedFeatureList feature_list_;
};
IN_PROC_BROWSER_TEST_F(TabSearchBubbleBrowserTest, TestShowAndCloseBubble) {
IN_PROC_BROWSER_TEST_F(TabSearchBubbleBrowserTest, ShowTriggersTimer) {
EXPECT_NE(nullptr, bubble());
// Test showing the bubble via ShowBubble() method.
// Timer should not be active when the bubble is not visible.
EXPECT_FALSE(bubble()->IsVisible());
bubble_view()->ShowBubble();
EXPECT_TRUE(bubble()->IsVisible());
// Test closing the bubble via CloseBubble() method.
EXPECT_FALSE(bubble()->IsClosed());
bubble_view()->CloseBubble();
EXPECT_TRUE(bubble()->IsClosed());
bubble()->CloseNow();
}
EXPECT_FALSE(bubble_view()->timer_for_testing());
IN_PROC_BROWSER_TEST_F(TabSearchBubbleBrowserTest, TestBubbleResize) {
EXPECT_NE(nullptr, bubble());
// Show the bubble
EXPECT_FALSE(bubble()->IsVisible());
bubble_view()->ShowBubble();
// Showing the bubble should trigger the timer.
bubble_view()->ShowUI();
EXPECT_TRUE(bubble()->IsVisible());
views::WebView* const web_view = bubble_view()->web_view_for_testing();
constexpr gfx::Size web_view_initial_size(100, 100);
web_view->SetPreferredSize(gfx::Size(100, 100));
bubble_view()->OnWebViewSizeChanged();
const gfx::Size widget_initial_size =
bubble()->GetWindowBoundsInScreen().size();
// The bubble should be at least as big as the webview.
EXPECT_GE(widget_initial_size.width(), web_view_initial_size.width());
EXPECT_GE(widget_initial_size.height(), web_view_initial_size.height());
// Resize the webview.
constexpr gfx::Size web_view_final_size(200, 200);
web_view->SetPreferredSize(web_view_final_size);
bubble_view()->OnWebViewSizeChanged();
// Ensure the bubble resizes as expected.
const gfx::Size widget_final_size =
bubble()->GetWindowBoundsInScreen().size();
EXPECT_LT(widget_initial_size.width(), widget_final_size.width());
EXPECT_LT(widget_initial_size.height(), widget_final_size.height());
// The bubble should be at least as big as the webview.
EXPECT_GE(widget_final_size.width(), web_view_final_size.width());
EXPECT_GE(widget_final_size.height(), web_view_final_size.height());
EXPECT_TRUE(bubble_view()->timer_for_testing());
bubble()->CloseNow();
}
......@@ -100,7 +65,10 @@ IN_PROC_BROWSER_TEST_F(TabSearchBubbleBrowserTest, TestBubbleResize) {
class TabSearchBubbleBrowserUITest : public DialogBrowserTest {
public:
TabSearchBubbleBrowserUITest() {
feature_list_.InitAndEnableFeature(features::kTabSearch);
feature_list_.InitWithFeatures(
/*enabled_features=*/{features::kTabSearch,
features::kTabSearchFixedEntrypoint},
/*disabled_features=*/{});
}
TabSearchBubbleBrowserUITest(TabSearchBubbleBrowserUITest&) = delete;
TabSearchBubbleBrowserUITest& operator=(TabSearchBubbleBrowserUITest&) =
......
......@@ -34,12 +34,12 @@ TabSearchPageHandler::TabSearchPageHandler(
mojo::PendingReceiver<tab_search::mojom::PageHandler> receiver,
mojo::PendingRemote<tab_search::mojom::Page> page,
content::WebUI* web_ui,
Delegate* delegate)
ui::MojoBubbleWebUIController* webui_controller)
: receiver_(this, std::move(receiver)),
page_(std::move(page)),
browser_(chrome::FindLastActive()),
web_ui_(web_ui),
delegate_(delegate),
webui_controller_(webui_controller),
debounce_timer_(std::make_unique<base::RetainingOneShotTimer>(
FROM_HERE,
kTabsChangeDelay,
......@@ -155,11 +155,9 @@ void TabSearchPageHandler::SwitchToTab(
}
void TabSearchPageHandler::ShowUI() {
delegate_->ShowUI();
}
void TabSearchPageHandler::CloseUI() {
delegate_->CloseUI();
auto embedder = webui_controller_->embedder();
if (embedder)
embedder->ShowUI();
}
tab_search::mojom::TabPtr TabSearchPageHandler::GetTabData(
......
......@@ -17,7 +17,7 @@
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "ui/webui/mojo_web_ui_controller.h"
#include "ui/webui/mojo_bubble_web_ui_controller.h"
class Browser;
......@@ -33,17 +33,11 @@ class TabSearchPageHandler : public tab_search::mojom::PageHandler,
public TabStripModelObserver,
public BrowserTabStripTrackerDelegate {
public:
class Delegate {
public:
virtual void ShowUI() = 0;
virtual void CloseUI() = 0;
};
TabSearchPageHandler(
mojo::PendingReceiver<tab_search::mojom::PageHandler> receiver,
mojo::PendingRemote<tab_search::mojom::Page> page,
content::WebUI* web_ui,
Delegate* delegate);
ui::MojoBubbleWebUIController* webui_controller);
TabSearchPageHandler(const TabSearchPageHandler&) = delete;
TabSearchPageHandler& operator=(const TabSearchPageHandler&) = delete;
~TabSearchPageHandler() override;
......@@ -56,7 +50,9 @@ class TabSearchPageHandler : public tab_search::mojom::PageHandler,
void SwitchToTab(
tab_search::mojom::SwitchToTabInfoPtr switch_to_tab_info) override;
void ShowUI() override;
void CloseUI() override;
// TODO(tluk): Remove this once all uses of the CloseUI() interface are
// removed from the Tab Search WebUI code.
void CloseUI() override {}
// TabStripModelObserver:
void OnTabStripModelChanged(
......@@ -101,7 +97,7 @@ class TabSearchPageHandler : public tab_search::mojom::PageHandler,
mojo::Remote<tab_search::mojom::Page> page_;
Browser* const browser_;
content::WebUI* const web_ui_;
Delegate* const delegate_;
ui::MojoBubbleWebUIController* const webui_controller_;
BrowserTabStripTracker browser_tab_strip_tracker_{this, this};
std::unique_ptr<base::RetainingOneShotTimer> debounce_timer_;
......
......@@ -82,12 +82,12 @@ class TestTabSearchPageHandler : public TabSearchPageHandler {
public:
TestTabSearchPageHandler(mojo::PendingRemote<tab_search::mojom::Page> page,
content::WebUI* web_ui,
TabSearchPageHandler::Delegate* delegate)
ui::MojoBubbleWebUIController* webui_controller)
: TabSearchPageHandler(
mojo::PendingReceiver<tab_search::mojom::PageHandler>(),
std::move(page),
web_ui,
delegate) {
webui_controller) {
mock_debounce_timer_ = new base::MockRetainingOneShotTimer();
SetTimerForTesting(base::WrapUnique(mock_debounce_timer_));
}
......@@ -99,15 +99,6 @@ class TestTabSearchPageHandler : public TabSearchPageHandler {
base::MockRetainingOneShotTimer* mock_debounce_timer_;
};
class MockTabSearchPageHandlerDelegate : public TabSearchPageHandler::Delegate {
public:
MockTabSearchPageHandlerDelegate() = default;
virtual ~MockTabSearchPageHandlerDelegate() = default;
MOCK_METHOD(void, ShowUI, (), (override));
MOCK_METHOD(void, CloseUI, (), (override));
};
class TabSearchPageHandlerTest : public BrowserWithTestWindowTest {
public:
void SetUp() override {
......@@ -120,9 +111,10 @@ class TabSearchPageHandlerTest : public BrowserWithTestWindowTest {
CreateTestBrowser(browser()->profile()->GetPrimaryOTRProfile(), false);
browser4_ = CreateTestBrowser(profile2(), false);
BrowserList::SetLastActive(browser1());
handler_delegate_ = std::make_unique<MockTabSearchPageHandlerDelegate>();
webui_controller_ =
std::make_unique<ui::MojoBubbleWebUIController>(web_ui());
handler_ = std::make_unique<TestTabSearchPageHandler>(
page_.BindAndGetRemote(), web_ui(), handler_delegate_.get());
page_.BindAndGetRemote(), web_ui(), webui_controller_.get());
}
void TearDown() override {
......@@ -153,9 +145,6 @@ class TabSearchPageHandlerTest : public BrowserWithTestWindowTest {
Browser* browser4() { return browser4_.get(); }
TestTabSearchPageHandler* handler() { return handler_.get(); }
MockTabSearchPageHandlerDelegate* handler_delegate() {
return handler_delegate_.get();
}
void FireTimer() { handler_->mock_debounce_timer()->Fire(); }
bool IsTimerRunning() { return handler_->mock_debounce_timer()->IsRunning(); }
......@@ -188,7 +177,7 @@ class TabSearchPageHandlerTest : public BrowserWithTestWindowTest {
std::unique_ptr<Browser> browser3_;
std::unique_ptr<Browser> browser4_;
std::unique_ptr<TestTabSearchPageHandler> handler_;
std::unique_ptr<MockTabSearchPageHandlerDelegate> handler_delegate_;
std::unique_ptr<ui::MojoBubbleWebUIController> webui_controller_;
};
TEST_F(TabSearchPageHandlerTest, GetTabs) {
......@@ -355,16 +344,4 @@ TEST_F(TabSearchPageHandlerTest, MAYBE_ShowFeedbackPage) {
histogram_tester.ExpectTotalCount("Feedback.RequestSource", 1);
}
// Make sure the delegate receives the ShowUI() call.
TEST_F(TabSearchPageHandlerTest, ShowUITest) {
EXPECT_CALL(*handler_delegate(), ShowUI()).Times(1);
handler()->ShowUI();
}
// Make sure the delegate receives the closeUI() call.
TEST_F(TabSearchPageHandlerTest, CloseUITest) {
EXPECT_CALL(*handler_delegate(), CloseUI()).Times(1);
handler()->CloseUI();
}
} // namespace
......@@ -33,8 +33,8 @@ constexpr char kGeneratedPath[] =
#endif // BUILDFLAG(ENABLE_TAB_SEARCH)
TabSearchUI::TabSearchUI(content::WebUI* web_ui)
: ui::MojoWebUIController(web_ui,
true /* Needed for webui browser tests */),
: ui::MojoBubbleWebUIController(web_ui,
true /* Needed for webui browser tests */),
webui_load_timer_(web_ui->GetWebContents(),
"Tabs.TabSearch.WebUI.LoadDocumentTime",
"Tabs.TabSearch.WebUI.LoadCompletedTime") {
......@@ -100,22 +100,6 @@ void TabSearchUI::BindInterface(
page_factory_receiver_.Bind(std::move(receiver));
}
void TabSearchUI::SetEmbedder(TabSearchUIEmbedder* embedder) {
// Setting the embedder must be done before the page handler is created.
DCHECK(!embedder || !page_handler_);
embedder_ = embedder;
}
void TabSearchUI::ShowUI() {
if (embedder_)
embedder_->ShowBubble();
}
void TabSearchUI::CloseUI() {
if (embedder_)
embedder_->CloseBubble();
}
void TabSearchUI::CreatePageHandler(
mojo::PendingRemote<tab_search::mojom::Page> page,
mojo::PendingReceiver<tab_search::mojom::PageHandler> receiver) {
......
......@@ -7,20 +7,17 @@
#include <memory>
#include "base/callback_forward.h"
#include "base/macros.h"
#include "chrome/browser/ui/webui/tab_search/tab_search.mojom.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_page_handler.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_ui_embedder.h"
#include "chrome/browser/ui/webui/webui_load_timer.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "ui/webui/mojo_web_ui_controller.h"
#include "ui/webui/mojo_bubble_web_ui_controller.h"
class TabSearchUI : public ui::MojoWebUIController,
public tab_search::mojom::PageHandlerFactory,
public TabSearchPageHandler::Delegate {
class TabSearchUI : public ui::MojoBubbleWebUIController,
public tab_search::mojom::PageHandlerFactory {
public:
explicit TabSearchUI(content::WebUI* web_ui);
TabSearchUI(const TabSearchUI&) = delete;
......@@ -31,11 +28,6 @@ class TabSearchUI : public ui::MojoWebUIController,
// interface passing the pending receiver that will be internally bound.
void BindInterface(
mojo::PendingReceiver<tab_search::mojom::PageHandlerFactory> receiver);
void SetEmbedder(TabSearchUIEmbedder* embedder);
// TabSearchPageHandler::Delegate:
void ShowUI() override;
void CloseUI() override;
private:
// tab_search::mojom::PageHandlerFactory
......@@ -50,8 +42,6 @@ class TabSearchUI : public ui::MojoWebUIController,
WebuiLoadTimer webui_load_timer_;
TabSearchUIEmbedder* embedder_ = nullptr;
WEB_UI_CONTROLLER_TYPE_DECL();
};
......
// Copyright 2020 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_WEBUI_TAB_SEARCH_TAB_SEARCH_UI_EMBEDDER_H_
#define CHROME_BROWSER_UI_WEBUI_TAB_SEARCH_TAB_SEARCH_UI_EMBEDDER_H_
// Interface to be implemented by the embedder. Provides native UI
// functionality such as showing and closing a bubble view.
class TabSearchUIEmbedder {
public:
TabSearchUIEmbedder() = default;
virtual ~TabSearchUIEmbedder() = default;
virtual void ShowBubble() = 0;
virtual void CloseBubble() = 0;
};
#endif // CHROME_BROWSER_UI_WEBUI_TAB_SEARCH_TAB_SEARCH_UI_EMBEDDER_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment