Commit ce5d9da0 authored by Christopher Lam's avatar Christopher Lam Committed by Commit Bot

[desktop-pwas] Disable animations and layers on buttons in immersive mode.

This CL fixes an issue where buttons were not visible in immersive mode
due to being painted into layers. This has been fixed by disabling the
layers when entering immersive mode, which disables animations, but keeps
the buttons visible.

Bug: 787640
Change-Id: Ie34f71a30b7a3f995d01e11fdeb66ce49ab12c0f
Reviewed-on: https://chromium-review.googlesource.com/1004155
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550064}
parent b03b8007
......@@ -124,6 +124,10 @@ void HostedAppButtonContainer::DisableAnimationForTesting() {
g_animation_disabled_for_testing = true;
}
views::View* HostedAppButtonContainer::GetContentSettingContainerForTesting() {
return content_settings_container_;
}
const std::vector<ContentSettingImageView*>&
HostedAppButtonContainer::GetContentSettingViewsForTesting() const {
return content_settings_container_->GetContentSettingViewsForTesting();
......@@ -196,9 +200,15 @@ HostedAppButtonContainer::HostedAppButtonContainer(BrowserView* browser_view,
AddChildView(app_menu_button_);
browser_view_->SetToolbarButtonProvider(this);
browser_view_->immersive_mode_controller()->AddObserver(this);
}
HostedAppButtonContainer::~HostedAppButtonContainer() {}
HostedAppButtonContainer::~HostedAppButtonContainer() {
ImmersiveModeController* immersive_controller =
browser_view_->immersive_mode_controller();
if (immersive_controller)
immersive_controller->RemoveObserver(this);
}
void HostedAppButtonContainer::RefreshContentSettingViews() {
content_settings_container_->RefreshContentSettingViews();
......@@ -233,6 +243,28 @@ void HostedAppButtonContainer::ChildPreferredSizeChanged(views::View* child) {
PreferredSizeChanged();
}
void HostedAppButtonContainer::OnImmersiveRevealStarted() {
// Cancel the content setting animation as icons need immediately show in
// immersive mode.
if (fade_in_content_setting_buttons_timer_.IsRunning()) {
fade_in_content_setting_buttons_timer_.AbandonAndStop();
content_settings_container_->SetVisible(true);
}
// Remove layers so that buttons display correctly when painted into the
// immersive mode top container view.
// See https://crbug.com/787640 for details.
// TODO(calamity): Make immersive mode support button layers.
content_settings_container_->DestroyLayer();
// Disable the ink drop as ink drops also render layers.
app_menu_button_->SetInkDropMode(HostedAppMenuButton::InkDropMode::OFF);
}
void HostedAppButtonContainer::OnImmersiveFullscreenExited() {
content_settings_container_->SetPaintToLayer();
content_settings_container_->layer()->SetFillsBoundsOpaquely(false);
app_menu_button_->SetInkDropMode(HostedAppMenuButton::InkDropMode::ON);
}
void HostedAppButtonContainer::ChildVisibilityChanged(views::View* child) {
// Changes to layout need to be taken into account by the frame view.
PreferredSizeChanged();
......
......@@ -8,6 +8,7 @@
#include <memory>
#include "base/macros.h"
#include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
#include "chrome/browser/ui/views/location_bar/content_setting_image_view.h"
#include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
......@@ -31,7 +32,8 @@ class MenuButton;
// A container for hosted app buttons in the title bar.
class HostedAppButtonContainer : public views::AccessiblePaneView,
public BrowserActionsContainer::Delegate,
public ToolbarButtonProvider {
public ToolbarButtonProvider,
public ImmersiveModeController::Observer {
public:
// |active_icon_color| and |inactive_icon_color| indicate the colors to use
// for button icons when the window is focused and blurred respectively.
......@@ -51,12 +53,16 @@ class HostedAppButtonContainer : public views::AccessiblePaneView,
void StartTitlebarAnimation(base::TimeDelta origin_text_slide_duration);
private:
FRIEND_TEST_ALL_PREFIXES(ImmersiveModeControllerAshHostedAppBrowserTest,
FrameLayout);
friend class HostedAppNonClientFrameViewAshTest;
static void DisableAnimationForTesting();
class ContentSettingsContainer;
views::View* GetContentSettingContainerForTesting();
const std::vector<ContentSettingImageView*>&
GetContentSettingViewsForTesting() const;
......@@ -80,6 +86,10 @@ class HostedAppButtonContainer : public views::AccessiblePaneView,
void FocusToolbar() override;
views::AccessiblePaneView* GetAsAccessiblePaneView() override;
// ImmersiveModeController::Observer:
void OnImmersiveRevealStarted() override;
void OnImmersiveFullscreenExited() override;
// The containing browser view.
BrowserView* browser_view_;
......
......@@ -60,6 +60,9 @@ class ImmersiveModeController {
// Called when the immersive mode controller has been destroyed.
virtual void OnImmersiveModeControllerDestroyed() {}
// Called when immersive mode is exited.
virtual void OnImmersiveFullscreenExited() {}
protected:
virtual ~Observer() {}
};
......
......@@ -288,6 +288,8 @@ void ImmersiveModeControllerAsh::OnImmersiveFullscreenExited() {
DestroyMashRevealWidget();
browser_view_->top_container()->DestroyLayer();
LayoutBrowserRootView();
for (Observer& observer : observers_)
observer.OnImmersiveFullscreenExited();
}
void ImmersiveModeControllerAsh::SetVisibleFraction(double visible_fraction) {
......
......@@ -10,30 +10,37 @@
#include "ash/shell.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "base/macros.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
#include "chrome/browser/ui/exclusive_access/fullscreen_controller_test.h"
#include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
#include "chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/hosted_app_button_container.h"
#include "chrome/browser/ui/views/frame/hosted_app_menu_button.h"
#include "chrome/browser/ui/views/frame/immersive_mode_controller_ash.h"
#include "chrome/browser/ui/views/frame/top_container_view.h"
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "chrome/common/web_application_info.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/views/animation/test/ink_drop_host_view_test_api.h"
class ImmersiveModeControllerAshHostedAppBrowserTest
: public InProcessBrowserTest {
: public ExtensionBrowserTest {
public:
ImmersiveModeControllerAshHostedAppBrowserTest() = default;
~ImmersiveModeControllerAshHostedAppBrowserTest() override = default;
// InProcessBrowserTest override:
void SetUpOnMainThread() override {
Browser::CreateParams params = Browser::CreateParams::CreateForApp(
"test_browser_app", true /* trusted_source */, gfx::Rect(),
InProcessBrowserTest::browser()->profile(), true);
browser_ = new Browser(params);
WebApplicationInfo web_app_info;
web_app_info.app_url = GURL("https://example.org");
web_app_info.theme_color = SK_ColorBLUE;
const extensions::Extension* app = InstallBookmarkApp(web_app_info);
browser_ = LaunchAppBrowser(app);
controller_ = browser_view()->immersive_mode_controller();
// Disable animations in immersive fullscreen before we show the window,
......@@ -96,9 +103,9 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshHostedAppBrowserTest, Layout) {
ASSERT_FALSE(browser_view()->GetWidget()->IsFullscreen());
ASSERT_FALSE(controller()->IsEnabled());
// The tabstrip and toolbar are not visible for hosted apps.
// The tabstrip is not visible for hosted apps.
EXPECT_FALSE(tabstrip->visible());
EXPECT_FALSE(toolbar->visible());
EXPECT_TRUE(toolbar->visible());
// The window header should be above the web contents.
int header_height = GetBoundsInWidget(contents_web_view).y();
......@@ -119,10 +126,10 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshHostedAppBrowserTest, Layout) {
// Reveal the window header.
AttemptReveal();
// The tabstrip and toolbar should still be hidden and the web contents should
// still be flush with the top of the screen.
// The tabstrip should still be hidden and the web contents should still be
// flush with the top of the screen.
EXPECT_FALSE(tabstrip->visible());
EXPECT_FALSE(toolbar->visible());
EXPECT_TRUE(toolbar->visible());
EXPECT_EQ(0, GetBoundsInWidget(contents_web_view).y());
// During an immersive reveal, the window header should be painted to the
......@@ -138,7 +145,7 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshHostedAppBrowserTest, Layout) {
EXPECT_FALSE(browser_view()->GetWidget()->IsFullscreen());
EXPECT_FALSE(controller()->IsEnabled());
EXPECT_FALSE(tabstrip->visible());
EXPECT_FALSE(toolbar->visible());
EXPECT_TRUE(toolbar->visible());
EXPECT_EQ(header_height, GetBoundsInWidget(contents_web_view).y());
}
......@@ -220,6 +227,16 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshHostedAppBrowserTest,
EXPECT_FALSE(frame_test_api.size_button()->visible());
// Button layers in the browser frame are disabled in immersive mode so that
// buttons render correctly.
HostedAppButtonContainer* container =
frame_view->hosted_app_button_container_;
EXPECT_FALSE(container->GetContentSettingContainerForTesting()->layer());
views::test::InkDropHostViewTestApi ink_drop_api(container->app_menu_button_);
EXPECT_EQ(ink_drop_api.ink_drop_mode(),
views::InkDropHostView::InkDropMode::OFF);
EXPECT_FALSE(container->app_menu_button_->layer());
// Verify the size button is visible in clamshell mode, and that it does not
// cover the other two buttons.
tablet_mode_controller->EnableTabletModeWindowManager(false);
......@@ -231,4 +248,9 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshHostedAppBrowserTest,
frame_test_api.close_button()->GetBoundsInScreen()));
EXPECT_FALSE(frame_test_api.size_button()->GetBoundsInScreen().Intersects(
frame_test_api.minimize_button()->GetBoundsInScreen()));
// Button layers are re-enabled when immersive mode is exited.
EXPECT_TRUE(container->GetContentSettingContainerForTesting()->layer());
EXPECT_EQ(ink_drop_api.ink_drop_mode(),
views::InkDropHostView::InkDropMode::ON);
}
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