Skip to content

Commit

Permalink
Hide the caption buttons of a browser window launched in tablet mode
Browse files Browse the repository at this point in the history
Before the present CL, the caption buttons of a browser window would be
hidden most of the time, but not at the moment when a browser window is
created in tablet mode. Workflows such as entering and exiting overview
would then cause the browser window caption buttons to be hidden because
they were supposed to be hidden all along. As of the present CL, the
caption buttons really will be hidden in the first place.

The present CL adds a very flaky unit test. Without the present CL, the
related test TabletModeBrowserCaptionButtonVisibility is already a
little bit flaky, meaning that the present CL is not really introducing
a new problem. The present CL just adds to the priority of addressing
the preexisting test flake issue. See Issue 993974.

Test: browser_tests HomeLauncherBrowserNonClientFrameViewAshTest.CaptionButtonVisibilityForBrowserLaunchedInTabletMode
Bug: 993594, 993974
Change-Id: I63442abd452eba4ea6b35ad67273dec000ea1c70
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1754239
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687288}
  • Loading branch information
Avery Musbach authored and Commit Bot committed Aug 15, 2019
1 parent 3d7f9a1 commit 4ef8d20
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ void BrowserNonClientFrameViewAsh::GetWindowMask(const gfx::Size& size,

void BrowserNonClientFrameViewAsh::ResetWindowControls() {
BrowserNonClientFrameView::ResetWindowControls();
caption_button_container_->SetVisible(true);
caption_button_container_->SetVisible(ShouldShowCaptionButtons());
caption_button_container_->ResetWindowControls();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ class BrowserNonClientFrameViewAsh
FrameLayoutToggleTabletMode);
FRIEND_TEST_ALL_PREFIXES(HomeLauncherBrowserNonClientFrameViewAshTest,
TabletModeBrowserCaptionButtonVisibility);
FRIEND_TEST_ALL_PREFIXES(
HomeLauncherBrowserNonClientFrameViewAshTest,
CaptionButtonVisibilityForBrowserLaunchedInTabletMode);
FRIEND_TEST_ALL_PREFIXES(HomeLauncherBrowserNonClientFrameViewAshTest,
TabletModeAppCaptionButtonVisibility);
FRIEND_TEST_ALL_PREFIXES(NonHomeLauncherBrowserNonClientFrameViewAshTest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,20 @@ IN_PROC_BROWSER_TEST_P(HomeLauncherBrowserNonClientFrameViewAshTest,
EXPECT_TRUE(frame_view->caption_button_container_->GetVisible());
}

// TODO(crbug.com/993974): When the test flake has been addressed, improve
// performance by consolidating this unit test with
// |TabletModeBrowserCaptionButtonVisibility|. Do not forget to remove the
// corresponding |FRIEND_TEST_ALL_PREFIXES| usage from
// |BrowserNonClientFrameViewAsh|.
IN_PROC_BROWSER_TEST_P(HomeLauncherBrowserNonClientFrameViewAshTest,
CaptionButtonVisibilityForBrowserLaunchedInTabletMode) {
ASSERT_NO_FATAL_FAILURE(
ash::ShellTestApi().SetTabletModeEnabledForTest(true));
EXPECT_FALSE(GetFrameViewAsh(BrowserView::GetBrowserViewForBrowser(
CreateBrowser(browser()->profile())))
->caption_button_container_->GetVisible());
}

IN_PROC_BROWSER_TEST_P(HomeLauncherBrowserNonClientFrameViewAshTest,
TabletModeAppCaptionButtonVisibility) {
browser()->window()->Close();
Expand Down

0 comments on commit 4ef8d20

Please sign in to comment.