Skip to content

Commit

Permalink
[tab search] Add entrypoint to the ChromeOS browser frame
Browse files Browse the repository at this point in the history
This CL adds an option to position the tab search entrypoint besides
the browser frame caption buttons on ChromeOS. If enabled this will
replace the tab search entrypoint in the tab strip for ChromeOS
platforms.

This new entrypoint will be gated behind the
kChromeOSTabSearchCaptionButton feature flag.

Bug: 1227272
Change-Id: Ib91406f40830770a6eb985cf2e2533c4f74ed436
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3012061
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#904826}
  • Loading branch information
tom authored and Chromium LUCI CQ committed Jul 23, 2021
1 parent 548719c commit ba90ace
Show file tree
Hide file tree
Showing 16 changed files with 206 additions and 14 deletions.
1 change: 1 addition & 0 deletions ash/assistant/assistant_web_view_delegate_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class AssistantWebContainerCaptionButtonModel
case views::CAPTION_BUTTON_ICON_ZOOM:
case views::CAPTION_BUTTON_ICON_LOCATION:
case views::CAPTION_BUTTON_ICON_CENTER:
case views::CAPTION_BUTTON_ICON_CUSTOM:
case views::CAPTION_BUTTON_ICON_COUNT:
return false;
}
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2933,6 +2933,8 @@ static_library("ui") {
"views/frame/browser_non_client_frame_view_chromeos.cc",
"views/frame/browser_non_client_frame_view_chromeos.h",
"views/frame/browser_non_client_frame_view_factory_chromeos.cc",
"views/frame/tab_search_frame_caption_button.cc",
"views/frame/tab_search_frame_caption_button.h",
"views/frame/top_controls_slide_controller_chromeos.cc",
"views/frame/top_controls_slide_controller_chromeos.h",
"views/platform_keys_certificate_selector_chromeos.cc",
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/ui_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,11 @@ const base::Feature kWebUITabStripNewTabButtonInTabStrip{
const base::Feature kWebUIFeedback{"WebUIFeedback",
base::FEATURE_DISABLED_BY_DEFAULT};

#if defined(OS_CHROMEOS)
const base::Feature kChromeOSTabSearchCaptionButton{
"ChromeOSTabSearchCaptionButton", base::FEATURE_DISABLED_BY_DEFAULT};
#endif

#if defined(OS_MAC)
// Enabled an experiment which increases the prominence to grant MacOS system
// location permission to Chrome when location permissions have already been
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/ui_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ extern const base::Feature kWebUITabStripNewTabButtonInTabStrip;

extern const base::Feature kWebUIFeedback;

#if defined(OS_CHROMEOS)
extern const base::Feature kChromeOSTabSearchCaptionButton;
#endif

// Cocoa to views migration.
#if defined(OS_MAC)
extern const base::Feature kLocationPermissionsExperiment;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "chrome/browser/ui/views/frame/browser_frame.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
#include "chrome/browser/ui/views/frame/tab_search_frame_caption_button.h"
#include "chrome/browser/ui/views/frame/tab_strip_region_view.h"
#include "chrome/browser/ui/views/frame/top_container_view.h"
#include "chrome/browser/ui/views/profiles/profile_indicator_icon.h"
Expand Down Expand Up @@ -116,12 +117,19 @@ BrowserNonClientFrameViewChromeOS::~BrowserNonClientFrameViewChromeOS() {
}

void BrowserNonClientFrameViewChromeOS::Init() {
Browser* browser = browser_view()->browser();

std::unique_ptr<TabSearchFrameCaptionButton> tab_search_button;
if (TabSearchFrameCaptionButton::IsTabSearchCaptionButtonEnabled(browser)) {
tab_search_button =
std::make_unique<TabSearchFrameCaptionButton>(browser->profile());
tab_search_bubble_host_ = tab_search_button->tab_search_bubble_host();
}

caption_button_container_ =
new chromeos::FrameCaptionButtonContainerView(frame());
AddChildView(std::make_unique<chromeos::FrameCaptionButtonContainerView>(
frame(), std::move(tab_search_button)));
caption_button_container_->UpdateCaptionButtonState(false /*=animate*/);
AddChildView(caption_button_container_);

Browser* browser = browser_view()->browser();

// Initializing the TabIconView is expensive, so only do it if we need to.
if (browser_view()->ShouldShowWindowIcon()) {
Expand Down Expand Up @@ -244,6 +252,11 @@ SkColor BrowserNonClientFrameViewChromeOS::GetCaptionColor(
return SkColorSetA(active_color, inactive_alpha_ratio * SK_AlphaOPAQUE);
}

TabSearchBubbleHost*
BrowserNonClientFrameViewChromeOS::GetTabSearchBubbleHost() {
return tab_search_bubble_host_;
}

gfx::Rect BrowserNonClientFrameViewChromeOS::GetBoundsForClientView() const {
// The ClientView must be flush with the top edge of the widget so that the
// web contents can take up the entire screen in immersive fullscreen (with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class BrowserNonClientFrameViewChromeOS
void UpdateThrobber(bool running) override;
bool CanUserExitFullscreen() const override;
SkColor GetCaptionColor(BrowserFrameActiveState active_state) const override;
TabSearchBubbleHost* GetTabSearchBubbleHost() override;

// views::NonClientFrameView:
gfx::Rect GetBoundsForClientView() const override;
Expand Down Expand Up @@ -148,6 +149,8 @@ class BrowserNonClientFrameViewChromeOS
TabletModeAppCaptionButtonVisibility);
FRIEND_TEST_ALL_PREFIXES(NonHomeLauncherBrowserNonClientFrameViewChromeOSTest,
HeaderHeightForSnappedBrowserInSplitView);
FRIEND_TEST_ALL_PREFIXES(TabSearchFrameCaptionButtonTest,
TabSearchBubbleHostTest);

friend class WebAppNonClientFrameViewAshTest;

Expand Down Expand Up @@ -212,6 +215,8 @@ class BrowserNonClientFrameViewChromeOS
chromeos::FrameCaptionButtonContainerView* caption_button_container_ =
nullptr;

TabSearchBubbleHost* tab_search_bubble_host_ = nullptr;

// For popups, the window icon.
TabIconView* window_icon_ = nullptr;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include "chrome/browser/ui/views/location_bar/zoom_bubble_view.h"
#include "chrome/browser/ui/views/page_action/page_action_icon_view.h"
#include "chrome/browser/ui/views/page_info/page_info_bubble_view_base.h"
#include "chrome/browser/ui/views/tab_search_bubble_host.h"
#include "chrome/browser/ui/views/tabs/tab.h"
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "chrome/browser/ui/views/toolbar/app_menu.h"
Expand Down Expand Up @@ -1576,6 +1577,43 @@ IN_PROC_BROWSER_TEST_P(HomeLauncherBrowserNonClientFrameViewChromeOSTest,
EXPECT_TRUE(frame_view->caption_button_container_->GetVisible());
}

namespace {

class TabSearchFrameCaptionButtonTest
: public TopChromeMdParamTest<InProcessBrowserTest> {
public:
TabSearchFrameCaptionButtonTest() = default;
TabSearchFrameCaptionButtonTest(const TabSearchFrameCaptionButtonTest&) =
delete;
TabSearchFrameCaptionButtonTest& operator=(
const TabSearchFrameCaptionButtonTest&) = delete;
~TabSearchFrameCaptionButtonTest() override = default;

void SetUp() override {
scoped_feature_list_.InitAndEnableFeature(
features::kChromeOSTabSearchCaptionButton);
TopChromeMdParamTest<InProcessBrowserTest>::SetUp();
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
};

} // namespace

IN_PROC_BROWSER_TEST_P(TabSearchFrameCaptionButtonTest,
TabSearchBubbleHostTest) {
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
BrowserNonClientFrameViewChromeOS* frame_view = GetFrameViewAsh(browser_view);
ASSERT_TRUE(browser()->is_type_normal());

chromeos::FrameCaptionButtonContainerView::TestApi test(
frame_view->caption_button_container_);
EXPECT_TRUE(test.custom_button());
EXPECT_EQ(browser_view->GetTabSearchBubbleHost()->button(),
test.custom_button());
}

#define INSTANTIATE_TEST_SUITE(name) \
INSTANTIATE_TEST_SUITE_P(All, name, ::testing::Values(false, true))

Expand All @@ -1586,3 +1624,4 @@ INSTANTIATE_TEST_SUITE(ImmersiveModeBrowserViewTest);
INSTANTIATE_TEST_SUITE(ImmersiveModeBrowserViewTestNoWebUiTabStrip);
INSTANTIATE_TEST_SUITE(WebAppNonClientFrameViewAshTest);
INSTANTIATE_TEST_SUITE(HomeLauncherBrowserNonClientFrameViewChromeOSTest);
INSTANTIATE_TEST_SUITE(TabSearchFrameCaptionButtonTest);
40 changes: 40 additions & 0 deletions chrome/browser/ui/views/frame/tab_search_frame_caption_button.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2021 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.

#include "chrome/browser/ui/views/frame/tab_search_frame_caption_button.h"

#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/tab_search_bubble_host.h"
#include "chrome/grit/generated_resources.h"
#include "components/vector_icons/vector_icons.h"
#include "ui/base/hit_test.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/metadata/metadata_impl_macros.h"

TabSearchFrameCaptionButton::TabSearchFrameCaptionButton(Profile* profile)
: FrameCaptionButton(views::Button::PressedCallback(),
views::CAPTION_BUTTON_ICON_CUSTOM,
HTCLIENT),
tab_search_bubble_host_(
std::make_unique<TabSearchBubbleHost>(this, profile)) {
SetImage(views::CAPTION_BUTTON_ICON_CUSTOM,
views::FrameCaptionButton::Animate::kNo,
vector_icons::kCaretDownIcon);
SetTooltipText(l10n_util::GetStringUTF16(IDS_ACCNAME_TAB_SEARCH));
}

TabSearchFrameCaptionButton::~TabSearchFrameCaptionButton() = default;

// static.
bool TabSearchFrameCaptionButton::IsTabSearchCaptionButtonEnabled(
Browser* browser) {
return browser->is_type_normal() &&
base::FeatureList::IsEnabled(
features::kChromeOSTabSearchCaptionButton);
}

BEGIN_METADATA(TabSearchFrameCaptionButton, views::FrameCaptionButton)
END_METADATA
34 changes: 34 additions & 0 deletions chrome/browser/ui/views/frame/tab_search_frame_caption_button.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2021 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_FRAME_TAB_SEARCH_FRAME_CAPTION_BUTTON_H_
#define CHROME_BROWSER_UI_VIEWS_FRAME_TAB_SEARCH_FRAME_CAPTION_BUTTON_H_

#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/views/window/frame_caption_button.h"

class Browser;
class Profile;
class TabSearchBubbleHost;

class TabSearchFrameCaptionButton : public views::FrameCaptionButton {
public:
METADATA_HEADER(TabSearchFrameCaptionButton);
explicit TabSearchFrameCaptionButton(Profile* profile);
TabSearchFrameCaptionButton(const TabSearchFrameCaptionButton&) = delete;
TabSearchFrameCaptionButton& operator=(const TabSearchFrameCaptionButton&) =
delete;
~TabSearchFrameCaptionButton() override;

static bool IsTabSearchCaptionButtonEnabled(Browser* browser);

TabSearchBubbleHost* tab_search_bubble_host() {
return tab_search_bubble_host_.get();
}

private:
std::unique_ptr<TabSearchBubbleHost> tab_search_bubble_host_;
};

#endif // CHROME_BROWSER_UI_VIEWS_FRAME_TAB_SEARCH_FRAME_CAPTION_BUTTON_H_
5 changes: 5 additions & 0 deletions chrome/browser/ui/views/frame/tab_strip_region_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ TabStripRegionView::TabStripRegionView(std::unique_ptr<TabStrip> tab_strip) {
views::LayoutAlignment::kCenter);
tip_marquee_view_->SetProperty(views::kMarginsKey, control_padding);

#if defined(OS_CHROMEOS)
if (base::FeatureList::IsEnabled(features::kChromeOSTabSearchCaptionButton))
return;
#endif

#if defined(OS_WIN)
if (base::FeatureList::IsEnabled(features::kWin10TabSearchCaptionButton))
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,14 @@ class TabStripRegionViewBrowserTest : public InProcessBrowserTest {
~TabStripRegionViewBrowserTest() override = default;

void SetUp() override {
#if defined(OS_WIN)
// Ensure we run our tests with the tab search button placement configured
// for the tab strip region view.
#if defined(OS_CHROMEOS)
scoped_feature_list_.InitAndDisableFeature(
features::kChromeOSTabSearchCaptionButton);
#endif

#if defined(OS_WIN)
scoped_feature_list_.InitAndDisableFeature(
features::kWin10TabSearchCaptionButton);
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ class DefaultCaptionButtonModel : public CaptionButtonModel {
return frame_->widget_delegate()->CanResize();
case views::CAPTION_BUTTON_ICON_CLOSE:
return frame_->widget_delegate()->ShouldShowCloseButton();

case views::CAPTION_BUTTON_ICON_CUSTOM:
return true;
// No back or menu button by default.
case views::CAPTION_BUTTON_ICON_BACK:
case views::CAPTION_BUTTON_ICON_MENU:
Expand All @@ -148,7 +149,8 @@ class DefaultCaptionButtonModel : public CaptionButtonModel {
} // namespace

FrameCaptionButtonContainerView::FrameCaptionButtonContainerView(
views::Widget* frame)
views::Widget* frame,
std::unique_ptr<views::FrameCaptionButton> custom_button)
: views::AnimationDelegateViews(frame->GetRootView()),
frame_(frame),
model_(std::make_unique<DefaultCaptionButtonModel>(frame)) {
Expand All @@ -165,6 +167,9 @@ FrameCaptionButtonContainerView::FrameCaptionButtonContainerView(
}

// Insert the buttons left to right.
if (custom_button)
custom_button_ = AddChildView(std::move(custom_button));

menu_button_ = new views::FrameCaptionButton(
base::BindRepeating(&FrameCaptionButtonContainerView::MenuButtonPressed,
base::Unretained(this)),
Expand Down Expand Up @@ -221,6 +226,8 @@ void FrameCaptionButtonContainerView::SetButtonImage(
}

void FrameCaptionButtonContainerView::SetPaintAsActive(bool paint_as_active) {
if (custom_button_)
custom_button_->SetPaintAsActive(paint_as_active);
menu_button_->SetPaintAsActive(paint_as_active);
minimize_button_->SetPaintAsActive(paint_as_active);
size_button_->SetPaintAsActive(paint_as_active);
Expand All @@ -230,6 +237,8 @@ void FrameCaptionButtonContainerView::SetPaintAsActive(bool paint_as_active) {

void FrameCaptionButtonContainerView::SetBackgroundColor(
SkColor background_color) {
if (custom_button_)
custom_button_->SetBackgroundColor(background_color);
menu_button_->SetBackgroundColor(background_color);
minimize_button_->SetBackgroundColor(background_color);
size_button_->SetBackgroundColor(background_color);
Expand Down Expand Up @@ -258,6 +267,12 @@ void FrameCaptionButtonContainerView::UpdateCaptionButtonState(bool animate) {
size_button_->SetVisible(false);
}
}
if (custom_button_) {
custom_button_->SetEnabled(
model_->IsEnabled(views::CAPTION_BUTTON_ICON_CUSTOM));
custom_button_->SetVisible(
model_->IsVisible(views::CAPTION_BUTTON_ICON_CUSTOM));
}
size_button_->SetEnabled(
(model_->IsEnabled(views::CAPTION_BUTTON_ICON_MAXIMIZE_RESTORE) ||
model_->InZoomMode()));
Expand All @@ -279,6 +294,8 @@ void FrameCaptionButtonContainerView::UpdateSizeButtonTooltip(
}

void FrameCaptionButtonContainerView::SetButtonSize(const gfx::Size& size) {
if (custom_button_)
custom_button_->SetPreferredSize(size);
menu_button_->SetPreferredSize(size);
minimize_button_->SetPreferredSize(size);
size_button_->SetPreferredSize(size);
Expand Down Expand Up @@ -465,6 +482,8 @@ bool FrameCaptionButtonContainerView::IsMinimizeButtonVisible() const {
void FrameCaptionButtonContainerView::SetButtonsToNormal(Animate animate) {
SetButtonIcons(views::CAPTION_BUTTON_ICON_MINIMIZE,
views::CAPTION_BUTTON_ICON_CLOSE, animate);
if (custom_button_)
custom_button_->SetState(views::Button::STATE_NORMAL);
menu_button_->SetState(views::Button::STATE_NORMAL);
minimize_button_->SetState(views::Button::STATE_NORMAL);
size_button_->SetState(views::Button::STATE_NORMAL);
Expand All @@ -488,13 +507,14 @@ FrameCaptionButtonContainerView::GetButtonClosestTo(
gfx::Point position(position_in_screen);
views::View::ConvertPointFromScreen(this, &position);

views::FrameCaptionButton* buttons[] = {menu_button_, minimize_button_,
size_button_, close_button_};
views::FrameCaptionButton* buttons[] = {custom_button_, menu_button_,
minimize_button_, size_button_,
close_button_};
int min_squared_distance = INT_MAX;
views::FrameCaptionButton* closest_button = nullptr;
for (size_t i = 0; i < base::size(buttons); ++i) {
views::FrameCaptionButton* button = buttons[i];
if (!button->GetVisible())
if (!button || !button->GetVisible())
continue;

gfx::Point center_point = button->GetLocalBounds().CenterPoint();
Expand All @@ -513,10 +533,13 @@ FrameCaptionButtonContainerView::GetButtonClosestTo(
void FrameCaptionButtonContainerView::SetHoveredAndPressedButtons(
const views::FrameCaptionButton* to_hover,
const views::FrameCaptionButton* to_press) {
views::FrameCaptionButton* buttons[] = {menu_button_, minimize_button_,
size_button_, close_button_};
views::FrameCaptionButton* buttons[] = {custom_button_, menu_button_,
minimize_button_, size_button_,
close_button_};
for (size_t i = 0; i < base::size(buttons); ++i) {
views::FrameCaptionButton* button = buttons[i];
if (!button)
continue;
views::Button::ButtonState new_state = views::Button::STATE_NORMAL;
if (button == to_hover)
new_state = views::Button::STATE_HOVERED;
Expand Down
Loading

0 comments on commit ba90ace

Please sign in to comment.