Commit e8a9c63e authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

Remove ash::switches::kAshEnableV1AppBackButton

browser_non_client_frame_view_ash.cc includes
ash/public/cpp/ash_switches.h to make use of the
`kAshEnableV1AppBackButton` runtime switch. However, as
for the bug linked to its declaration, the support is
flagged for removal (https://crbug.com/749713 - WONTFIX'ed).

This CL removes it, and the code associated with it, including
if-blocks it was being checked and unittests.

This is phase 2.3 in the design document [1].

[1] https://docs.google.com/document/d/1xHwcHrAiEaWuA4usGEqKZ9zm1H8SGk3WkS-Jf2Q_los

The idea is to move browser_non_client_frame_view_ash.cc|h away from
using "ash" symbols. Ultimately, this file will be used by lacros/chrome
builds.

BUG=749713, 1113900

Change-Id: Iab1a7766f0142e66abda34a265d1308b341b2328
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424637Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Antonio Gomes (GMT-4) <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#810219}
parent d30d7bfe
...@@ -43,10 +43,6 @@ const char kAshDeveloperShortcuts[] = "ash-dev-shortcuts"; ...@@ -43,10 +43,6 @@ const char kAshDeveloperShortcuts[] = "ash-dev-shortcuts";
const char kAshDisableTouchExplorationMode[] = const char kAshDisableTouchExplorationMode[] =
"ash-disable-touch-exploration-mode"; "ash-disable-touch-exploration-mode";
// Enables Backbutton on frame for v1 apps.
// TODO(oshima): Remove this once the feature is launched. crbug.com/749713.
const char kAshEnableV1AppBackButton[] = "ash-enable-v1-app-back-button";
// Enable cursor motion blur. // Enable cursor motion blur.
const char kAshEnableCursorMotionBlur[] = "ash-enable-cursor-motion-blur"; const char kAshEnableCursorMotionBlur[] = "ash-enable-cursor-motion-blur";
......
...@@ -25,7 +25,6 @@ ASH_PUBLIC_EXPORT extern const char kAshDebugShortcuts[]; ...@@ -25,7 +25,6 @@ ASH_PUBLIC_EXPORT extern const char kAshDebugShortcuts[];
ASH_PUBLIC_EXPORT extern const char kAshDeveloperShortcuts[]; ASH_PUBLIC_EXPORT extern const char kAshDeveloperShortcuts[];
ASH_PUBLIC_EXPORT extern const char kAshDisableTouchExplorationMode[]; ASH_PUBLIC_EXPORT extern const char kAshDisableTouchExplorationMode[];
ASH_PUBLIC_EXPORT extern const char kAshEnableCursorMotionBlur[]; ASH_PUBLIC_EXPORT extern const char kAshEnableCursorMotionBlur[];
ASH_PUBLIC_EXPORT extern const char kAshEnableV1AppBackButton[];
ASH_PUBLIC_EXPORT extern const char kAshEnableMagnifierKeyScroller[]; ASH_PUBLIC_EXPORT extern const char kAshEnableMagnifierKeyScroller[];
ASH_PUBLIC_EXPORT extern const char kAshEnablePaletteOnAllDisplays[]; ASH_PUBLIC_EXPORT extern const char kAshEnablePaletteOnAllDisplays[];
ASH_PUBLIC_EXPORT extern const char kAshEnableTabletMode[]; ASH_PUBLIC_EXPORT extern const char kAshEnableTabletMode[];
......
...@@ -8,8 +8,6 @@ ...@@ -8,8 +8,6 @@
#include "ash/public/cpp/app_types.h" #include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/ash_constants.h" #include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/ash_switches.h"
#include "ash/public/cpp/caption_buttons/frame_back_button.h"
#include "ash/public/cpp/caption_buttons/frame_caption_button_container_view.h" #include "ash/public/cpp/caption_buttons/frame_caption_button_container_view.h"
#include "ash/public/cpp/default_frame_header.h" #include "ash/public/cpp/default_frame_header.h"
#include "ash/public/cpp/frame_utils.h" #include "ash/public/cpp/frame_utils.h"
...@@ -17,18 +15,15 @@ ...@@ -17,18 +15,15 @@
#include "ash/public/cpp/window_properties.h" #include "ash/public/cpp/window_properties.h"
#include "ash/public/cpp/window_state_type.h" #include "ash/public/cpp/window_state_type.h"
#include "ash/wm/window_util.h" #include "ash/wm/window_util.h"
#include "base/command_line.h"
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/platform_util.h" #include "chrome/browser/platform_util.h"
#include "chrome/browser/profiles/profiles_state.h" #include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h" #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h"
#include "chrome/browser/ui/ash/session_util.h" #include "chrome/browser/ui/ash/session_util.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_command_controller.h"
#include "chrome/browser/ui/layout_constants.h" #include "chrome/browser/ui/layout_constants.h"
#include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/frame/browser_frame.h" #include "chrome/browser/ui/views/frame/browser_frame.h"
...@@ -77,11 +72,6 @@ constexpr SkColor kIncognitoWindowTitleTextColor = SK_ColorWHITE; ...@@ -77,11 +72,6 @@ constexpr SkColor kIncognitoWindowTitleTextColor = SK_ColorWHITE;
// The indicator for teleported windows has 8 DIPs before and below it. // The indicator for teleported windows has 8 DIPs before and below it.
constexpr int kProfileIndicatorPadding = 8; constexpr int kProfileIndicatorPadding = 8;
bool IsV1AppBackButtonEnabled() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(
ash::switches::kAshEnableV1AppBackButton);
}
// Returns true if the header should be painted so that it looks the same as // Returns true if the header should be painted so that it looks the same as
// the header used for packaged apps. // the header used for packaged apps.
bool UsePackagedAppHeaderStyle(const Browser* browser) { bool UsePackagedAppHeaderStyle(const Browser* browser) {
...@@ -104,12 +94,6 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh( ...@@ -104,12 +94,6 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh(
} }
BrowserNonClientFrameViewAsh::~BrowserNonClientFrameViewAsh() { BrowserNonClientFrameViewAsh::~BrowserNonClientFrameViewAsh() {
if (browser_view()->browser()->deprecated_is_app() &&
IsV1AppBackButtonEnabled()) {
browser_view()->browser()->command_controller()->RemoveCommandObserver(
IDC_BACK, this);
}
ash::TabletMode::Get()->RemoveObserver(this); ash::TabletMode::Get()->RemoveObserver(this);
ImmersiveModeController* immersive_controller = ImmersiveModeController* immersive_controller =
...@@ -150,13 +134,6 @@ void BrowserNonClientFrameViewAsh::Init() { ...@@ -150,13 +134,6 @@ void BrowserNonClientFrameViewAsh::Init() {
ash::TabletMode::Get()->AddObserver(this); ash::TabletMode::Get()->AddObserver(this);
if (browser->deprecated_is_app() && IsV1AppBackButtonEnabled()) {
browser->command_controller()->AddCommandObserver(IDC_BACK, this);
back_button_ = new ash::FrameBackButton();
AddChildView(back_button_);
// TODO(oshima): Add Tooltip, accessibility name.
}
if (frame()->ShouldDrawFrameHeader()) if (frame()->ShouldDrawFrameHeader())
frame_header_ = CreateFrameHeader(); frame_header_ = CreateFrameHeader();
...@@ -503,15 +480,6 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFaviconForTabIconView() { ...@@ -503,15 +480,6 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFaviconForTabIconView() {
return delegate ? delegate->GetWindowIcon() : gfx::ImageSkia(); return delegate ? delegate->GetWindowIcon() : gfx::ImageSkia();
} }
void BrowserNonClientFrameViewAsh::EnabledStateChangedForCommand(int id,
bool enabled) {
DCHECK_EQ(IDC_BACK, id);
DCHECK(browser_view()->browser()->deprecated_is_app());
if (back_button_)
back_button_->SetEnabled(enabled);
}
void BrowserNonClientFrameViewAsh::OnWindowDestroying(aura::Window* window) { void BrowserNonClientFrameViewAsh::OnWindowDestroying(aura::Window* window) {
window_observer_.RemoveAll(); window_observer_.RemoveAll();
} }
...@@ -552,8 +520,6 @@ void BrowserNonClientFrameViewAsh::OnImmersiveRevealStarted() { ...@@ -552,8 +520,6 @@ void BrowserNonClientFrameViewAsh::OnImmersiveRevealStarted() {
container->AddChildViewAt(caption_button_container_, 0); container->AddChildViewAt(caption_button_container_, 0);
if (web_app_frame_toolbar()) if (web_app_frame_toolbar())
container->AddChildViewAt(web_app_frame_toolbar(), 0); container->AddChildViewAt(web_app_frame_toolbar(), 0);
if (back_button_)
container->AddChildViewAt(back_button_, 0);
container->Layout(); container->Layout();
} }
...@@ -562,8 +528,6 @@ void BrowserNonClientFrameViewAsh::OnImmersiveRevealEnded() { ...@@ -562,8 +528,6 @@ void BrowserNonClientFrameViewAsh::OnImmersiveRevealEnded() {
AddChildViewAt(caption_button_container_, 0); AddChildViewAt(caption_button_container_, 0);
if (web_app_frame_toolbar()) if (web_app_frame_toolbar())
AddChildViewAt(web_app_frame_toolbar(), 0); AddChildViewAt(web_app_frame_toolbar(), 0);
if (back_button_)
AddChildViewAt(back_button_, 0);
Layout(); Layout();
} }
...@@ -660,7 +624,6 @@ BrowserNonClientFrameViewAsh::CreateFrameHeader() { ...@@ -660,7 +624,6 @@ BrowserNonClientFrameViewAsh::CreateFrameHeader() {
frame(), this, caption_button_container_); frame(), this, caption_button_container_);
} }
header->SetBackButton(back_button_);
header->SetLeftHeaderView(window_icon_); header->SetLeftHeaderView(window_icon_);
return header; return header;
} }
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#include "chrome/browser/command_observer.h"
#include "chrome/browser/ui/views/frame/browser_frame_header_ash.h" #include "chrome/browser/ui/views/frame/browser_frame_header_ash.h"
#include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h" #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
#include "chrome/browser/ui/views/frame/immersive_mode_controller.h" #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
...@@ -31,17 +30,12 @@ namespace ash { ...@@ -31,17 +30,12 @@ namespace ash {
class FrameCaptionButtonContainerView; class FrameCaptionButtonContainerView;
} // namespace ash } // namespace ash
namespace views {
class FrameCaptionButton;
} // namespace views
// Provides the BrowserNonClientFrameView for Chrome OS. // Provides the BrowserNonClientFrameView for Chrome OS.
class BrowserNonClientFrameViewAsh class BrowserNonClientFrameViewAsh
: public BrowserNonClientFrameView, : public BrowserNonClientFrameView,
public BrowserFrameHeaderAsh::AppearanceProvider, public BrowserFrameHeaderAsh::AppearanceProvider,
public ash::TabletModeObserver, public ash::TabletModeObserver,
public TabIconViewModel, public TabIconViewModel,
public CommandObserver,
public aura::WindowObserver, public aura::WindowObserver,
public ImmersiveModeController::Observer { public ImmersiveModeController::Observer {
public: public:
...@@ -97,9 +91,6 @@ class BrowserNonClientFrameViewAsh ...@@ -97,9 +91,6 @@ class BrowserNonClientFrameViewAsh
bool ShouldTabIconViewAnimate() const override; bool ShouldTabIconViewAnimate() const override;
gfx::ImageSkia GetFaviconForTabIconView() override; gfx::ImageSkia GetFaviconForTabIconView() override;
// CommandObserver:
void EnabledStateChangedForCommand(int id, bool enabled) override;
// aura::WindowObserver: // aura::WindowObserver:
void OnWindowDestroying(aura::Window* window) override; void OnWindowDestroying(aura::Window* window) override;
void OnWindowPropertyChanged(aura::Window* window, void OnWindowPropertyChanged(aura::Window* window,
...@@ -133,8 +124,6 @@ class BrowserNonClientFrameViewAsh ...@@ -133,8 +124,6 @@ class BrowserNonClientFrameViewAsh
AppHeaderVisibilityInTabletModeTest); AppHeaderVisibilityInTabletModeTest);
FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewAshTest, FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewAshTest,
ImmersiveModeTopViewInset); ImmersiveModeTopViewInset);
FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewAshBackButtonTest,
V1BackButton);
FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewAshTest, FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewAshTest,
ToggleTabletModeOnMinimizedWindow); ToggleTabletModeOnMinimizedWindow);
FRIEND_TEST_ALL_PREFIXES(WebAppNonClientFrameViewAshTest, FRIEND_TEST_ALL_PREFIXES(WebAppNonClientFrameViewAshTest,
...@@ -215,8 +204,6 @@ class BrowserNonClientFrameViewAsh ...@@ -215,8 +204,6 @@ class BrowserNonClientFrameViewAsh
// View which contains the window controls. // View which contains the window controls.
ash::FrameCaptionButtonContainerView* caption_button_container_ = nullptr; ash::FrameCaptionButtonContainerView* caption_button_container_ = nullptr;
views::FrameCaptionButton* back_button_ = nullptr;
// For popups, the window icon. // For popups, the window icon.
TabIconView* window_icon_ = nullptr; TabIconView* window_icon_ = nullptr;
......
...@@ -1272,61 +1272,6 @@ IN_PROC_BROWSER_TEST_P(WebAppNonClientFrameViewAshTest, PopupHasNoToolbar) { ...@@ -1272,61 +1272,6 @@ IN_PROC_BROWSER_TEST_P(WebAppNonClientFrameViewAshTest, PopupHasNoToolbar) {
EXPECT_FALSE(frame_view->web_app_frame_toolbar_for_testing()); EXPECT_FALSE(frame_view->web_app_frame_toolbar_for_testing());
} }
namespace {
class BrowserNonClientFrameViewAshBackButtonTest
: public TopChromeMdParamTest<InProcessBrowserTest> {
public:
BrowserNonClientFrameViewAshBackButtonTest() = default;
~BrowserNonClientFrameViewAshBackButtonTest() override = default;
void SetUpCommandLine(base::CommandLine* command_line) override {
command_line->AppendSwitch(ash::switches::kAshEnableV1AppBackButton);
}
private:
DISALLOW_COPY_AND_ASSIGN(BrowserNonClientFrameViewAshBackButtonTest);
};
} // namespace
// Test if the V1 apps' frame has a back button.
IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshBackButtonTest,
V1BackButton) {
// Normal browser windows don't have a frame back button.
BrowserNonClientFrameViewAsh* frame_view =
GetFrameViewAsh(BrowserView::GetBrowserViewForBrowser(browser()));
EXPECT_FALSE(frame_view->back_button_);
browser()->window()->Close();
// Open a new app window.
Browser::CreateParams params = Browser::CreateParams::CreateForApp(
"test_browser_app", true /* trusted_source */, gfx::Rect(),
browser()->profile(), true);
params.initial_show_state = ui::SHOW_STATE_DEFAULT;
Browser* app_browser = new Browser(params);
AddBlankTabAndShow(app_browser);
BrowserNonClientFrameViewAsh* app_frame_view =
GetFrameViewAsh(BrowserView::GetBrowserViewForBrowser(app_browser));
ASSERT_TRUE(app_frame_view->back_button_);
EXPECT_TRUE(app_frame_view->back_button_->GetVisible());
// The back button should be disabled initially.
EXPECT_FALSE(app_frame_view->back_button_->GetEnabled());
// Nagivate to a page. The back button should now be enabled.
const GURL kAppStartURL("http://example.org/");
NavigateParams nav_params(app_browser, kAppStartURL,
ui::PAGE_TRANSITION_LINK);
ui_test_utils::NavigateToURL(&nav_params);
EXPECT_TRUE(app_frame_view->back_button_->GetEnabled());
// Go back to the blank. The back button should be disabled again.
chrome::GoBack(app_browser, WindowOpenDisposition::CURRENT_TAB);
EXPECT_FALSE(app_frame_view->back_button_->GetEnabled());
}
// Test the normal type browser's kTopViewInset is always 0. // Test the normal type browser's kTopViewInset is always 0.
IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest, TopViewInset) { IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest, TopViewInset) {
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser()); BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
...@@ -1586,5 +1531,4 @@ INSTANTIATE_TEST_SUITE(BrowserNonClientFrameViewAshTestWithWebUiTabStrip); ...@@ -1586,5 +1531,4 @@ INSTANTIATE_TEST_SUITE(BrowserNonClientFrameViewAshTestWithWebUiTabStrip);
INSTANTIATE_TEST_SUITE(ImmersiveModeBrowserViewTest); INSTANTIATE_TEST_SUITE(ImmersiveModeBrowserViewTest);
INSTANTIATE_TEST_SUITE(ImmersiveModeBrowserViewTestNoWebUiTabStrip); INSTANTIATE_TEST_SUITE(ImmersiveModeBrowserViewTestNoWebUiTabStrip);
INSTANTIATE_TEST_SUITE(WebAppNonClientFrameViewAshTest); INSTANTIATE_TEST_SUITE(WebAppNonClientFrameViewAshTest);
INSTANTIATE_TEST_SUITE(BrowserNonClientFrameViewAshBackButtonTest);
INSTANTIATE_TEST_SUITE(HomeLauncherBrowserNonClientFrameViewAshTest); INSTANTIATE_TEST_SUITE(HomeLauncherBrowserNonClientFrameViewAshTest);
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