Cleanup pre-refresh functionality in tab.cc, part 1.

This removes support for pre-refresh modes.  It does not change the design of
path computations/drawing or other deeper refactors.

Also does some other minor cleanup, and fixes one clear bug with high DPI in
PaintChildren().

Bug: 873855
Change-Id: I3496ea60ed670d5f1c30abf33b989162c558fa16
Reviewed-on: https://chromium-review.googlesource.com/1231898Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592527}

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc

@@ -322,64 +322,12 @@ IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest,
   manager->ShowWindowForUser(window, account_id2);
   EXPECT_TRUE(MultiUserWindowManager::ShouldShowAvatar(window));
 
-  if (GetParam() != switches::kTopChromeMDMaterialRefresh) {
-    // An icon should show on the top left corner of the teleported browser
-    // window.
-    EXPECT_TRUE(frame_view->profile_indicator_icon());
-  }
-
   // Teleport the window back to owner desktop.
   manager->ShowWindowForUser(window, account_id1);
   EXPECT_FALSE(MultiUserWindowManager::ShouldShowAvatar(window));
   EXPECT_FALSE(frame_view->profile_indicator_icon());
 }
 
-// Hit Test for Avatar Menu Button on ChromeOS.
-IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest,
-                       AvatarMenuButtonHitTestOnChromeOS) {
-  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
-  BrowserNonClientFrameViewAsh* frame_view = GetFrameViewAsh(browser_view);
-
-  gfx::Point avatar_center(profiles::kAvatarIconWidth / 2,
-                           profiles::kAvatarIconHeight / 2);
-  // The increased header height in the touch-optimized UI affects the expected
-  // result.
-  int expected_value =
-      GetParam() == switches::kTopChromeMDMaterialTouchOptimized ? HTCAPTION
-                                                                 : HTCLIENT;
-  EXPECT_EQ(expected_value, frame_view->NonClientHitTest(avatar_center));
-  EXPECT_FALSE(frame_view->profile_indicator_icon());
-
-  const AccountId current_user =
-      multi_user_util::GetAccountIdFromProfile(browser()->profile());
-  TestMultiUserWindowManager* manager =
-      new TestMultiUserWindowManager(browser(), current_user);
-
-  // Teleport the window to another desktop.
-  const AccountId account_id2(AccountId::FromUserEmail("user2"));
-  manager->ShowWindowForUser(browser()->window()->GetNativeWindow(),
-                             account_id2);
-  if (GetParam() != switches::kTopChromeMDMaterialRefresh) {
-    // Clicking on the avatar icon should have same behaviour like clicking on
-    // the caption area, i.e., allow the user to drag the browser window around.
-    EXPECT_EQ(HTCAPTION, frame_view->NonClientHitTest(avatar_center));
-    EXPECT_TRUE(frame_view->profile_indicator_icon());
-  }
-}
-
-// Tests that for an incognito browser, there is an avatar icon view, unless in
-// touch-optimized mode.
-IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest, IncognitoAvatar) {
-  Browser* incognito_browser = CreateIncognitoBrowser();
-  BrowserView* browser_view =
-      BrowserView::GetBrowserViewForBrowser(incognito_browser);
-  BrowserNonClientFrameViewAsh* frame_view = GetFrameViewAsh(browser_view);
-
-  const bool should_have_avatar = GetParam() == switches::kTopChromeMDMaterial;
-  const bool has_avatar = !!frame_view->profile_indicator_icon();
-  EXPECT_EQ(should_have_avatar, has_avatar);
-}
-
 IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest,
                        IncognitoMarkedAsAssistantBlocked) {
   Browser* incognito_browser = CreateIncognitoBrowser();
@@ -1451,9 +1399,8 @@ IN_PROC_BROWSER_TEST_P(NonHomeLauncherBrowserNonClientFrameViewAshTest,
 #define INSTANTIATE_TEST_CASE(name)                                           \
   INSTANTIATE_TEST_CASE_P(                                                    \
       , name,                                                                 \
-      ::testing::Values(switches::kTopChromeMDMaterial,               \
-                        switches::kTopChromeMDMaterialTouchOptimized, \
-                        switches::kTopChromeMDMaterialRefresh),       \
+      ::testing::Values(switches::kTopChromeMDMaterialRefresh,                \
+                        switches::kTopChromeMDMaterialRefreshTouchOptimized), \
       &TopChromeMdParamToString)
 
 INSTANTIATE_TEST_CASE(BrowserNonClientFrameViewAshTest);

chrome/browser/ui/views/tabs/tab.h

@@ -54,8 +54,8 @@ class Tab : public gfx::AnimationDelegate,
   // The Tab's class name.
   static const char kViewClassName[];
 
-  // Under refresh, thickness in DIPs of the separator painted on the left and
-  // right edges of the tab.
+  // Thickness in DIPs of the separator painted on the left and right edges of
+  // the tab.
   static constexpr int kSeparatorThickness = 1;
 
   // When the content's width of the tab shrinks to below this size we should
@@ -123,9 +123,6 @@ class Tab : public gfx::AnimationDelegate,
   // Returns the color used for the alert indicator icon.
   SkColor GetAlertIndicatorColor(TabAlertState state) const;
 
-  // Returns the color to be used for the tab close button.
-  SkColor GetCloseTabButtonColor(views::Button::ButtonState button_state) const;
-
   // Returns true if this tab is the active tab.
   bool IsActive() const;
 
@@ -177,12 +174,15 @@ class Tab : public gfx::AnimationDelegate,
 
   bool mouse_hovered() const { return mouse_hovered_; }
 
-  // Returns the thickness of the stroke drawn around the tab.  If
-  // |should_paint_as_active| is true, the tab is treated as an active tab
-  // regardless of its true current state; this affects Refresh, which never
-  // paints strokes on inactive tabs.
+  // Returns the thickness of the stroke drawn around the top and sides of the
+  // tab.  Only active tabs may have a stroke, and not in all cases.  If there
+  // is no stroke, returns 0.  If |should_paint_as_active| is true, the tab is
+  // treated as an active tab regardless of its true current state.
   float GetStrokeThickness(bool should_paint_as_active = false) const;
 
+  // Returns the thickness of the stroke drawn below the tab.
+  float GetBottomStrokeThickness(bool should_paint_as_active = false) const;
+
   // Returns the width of the largest part of the tab that is available for the
   // user to click to select/activate the tab.
   int GetWidthOfLargestSelectableRegion() const;
@@ -274,8 +274,7 @@ class Tab : public gfx::AnimationDelegate,
                                 bool active,
                                 SkColor color);
 
-  // Paints the separator lines on the left and right edge of the tab if in
-  // material refresh mode.
+  // Paints the separator lines on the left and right edge of the tab.
   void PaintSeparators(gfx::Canvas* canvas);
 
   // Computes which icons are visible in the tab. Should be called everytime
@@ -299,11 +298,6 @@ class Tab : public gfx::AnimationDelegate,
   // tab title change and pulsing.
   float GetThrobValue() const;
 
-  // Recalculates the correct |button_color_| and resets the title, alert
-  // indicator, and close button colors if necessary.  This should be called any
-  // time the theme or active state may have changed.
-  void OnButtonColorMaybeChanged();
-
   // Updates the blocked attention state of the |icon_|. This only updates
   // state; it is the responsibility of the caller to request a paint.
   void UpdateTabIconNeedsAttentionBlocked();

chrome/browser/ui/views/tabs/tab_close_button.h

@@ -65,8 +65,8 @@ class TabCloseButton : public views::ImageButton,
   // Draw the close "X" glyph.
   void DrawCloseGlyph(gfx::Canvas* canvas, ButtonState state);
 
-  // In material refresh mode, calculates opacity based on the current state of
-  // the hover animation on the parent tab.
+  // Calculates opacity based on the current state of the hover animation on the
+  // parent tab.
   SkAlpha GetOpacity();
 
   MouseEventCallback mouse_event_callback_;

chrome/browser/ui/views/tabs/tab_controller.h

@@ -36,9 +36,9 @@ class TabController {
   // Returns true if multiple selection is supported.
   virtual bool SupportsMultipleSelection() = 0;
 
-  // Under Refresh, returns where the new tab button should be placed. This is
-  // needed to determine which tab separators need to be faded in/out while
-  // animating into position.
+  // Returns where the new tab button should be placed. This is needed to
+  // determine which tab separators need to be faded in/out while animating into
+  // position.
   virtual NewTabButtonPosition GetNewTabButtonPosition() const = 0;
 
   // Returns true if the close button for the given tab is forced to be hidden.
@@ -130,8 +130,8 @@ class TabController {
           border_callback,
       gfx::Path* clip) = 0;
 
-  // Returns the thickness of the stroke around all tabs (for pre-refresh) or
-  // the active tab (for refresh) in DIP.  Returns 0 if there is no stroke.
+  // Returns the thickness of the stroke around the active tab in DIP.  Returns
+  // 0 if there is no stroke.
   virtual int GetStrokeThickness() const = 0;
 
   // Returns true if tab loading throbbers can be painted to a composited layer.
@@ -147,7 +147,7 @@ class TabController {
   // state of the window.
   virtual SkColor GetToolbarTopSeparatorColor() const = 0;
 
-  // Under Refresh, returns the color of the separator between the tabs.
+  // Returns the color of the separator between the tabs.
   virtual SkColor GetTabSeparatorColor() const = 0;
 
   // Returns the tab background color based on both the |state| of the tab and

chrome/browser/ui/views/tabs/tab_strip_unittest.cc

@@ -186,15 +186,6 @@ class TabStripTest : public ChromeViewsTestBase,
     return delegate;
   }
 
-  bool IsTabShowingCloseButton(Tab* tab) {
-    ui::MouseEvent event(ui::ET_MOUSE_ENTERED, gfx::Point(0, 0),
-                         gfx::Point(0, 0), ui::EventTimeForNow(), 0, 0);
-    // In Refresh, close buttons on inactive tabs are never visible until the
-    // tab is hovered. It's harmless to do this in other cases.
-    tab->OnMouseEntered(event);
-    return tab->showing_close_button_;
-  }
-
   bool IsShowingAttentionIndicator(Tab* tab) {
     return tab->icon_->ShowingAttentionIndicator();
   }
@@ -443,9 +434,9 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenStacked) {
   Tab* tab2 = tab_strip_->tab_at(2);
 
   // Ensure that all tab close buttons are initially visible.
-  EXPECT_TRUE(IsTabShowingCloseButton(tab0));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab1));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab2));
+  EXPECT_TRUE(tab0->showing_close_button_);
+  EXPECT_TRUE(tab1->showing_close_button_);
+  EXPECT_TRUE(tab2->showing_close_button_);
 
   // Enter stacked layout mode and verify this sets |touch_layout_|.
   ASSERT_FALSE(touch_layout());
@@ -454,18 +445,18 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenStacked) {
 
   // Only the close button of the active tab should be visible in stacked
   // layout mode.
-  EXPECT_FALSE(IsTabShowingCloseButton(tab0));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab1));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab2));
+  EXPECT_FALSE(tab0->showing_close_button_);
+  EXPECT_TRUE(tab1->showing_close_button_);
+  EXPECT_FALSE(tab2->showing_close_button_);
 
   // An inactive tab added to the tabstrip should not show
   // its tab close button.
   controller_->AddTab(3, false);
   Tab* tab3 = tab_strip_->tab_at(3);
-  EXPECT_FALSE(IsTabShowingCloseButton(tab0));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab1));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab2));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab3));
+  EXPECT_FALSE(tab0->showing_close_button_);
+  EXPECT_TRUE(tab1->showing_close_button_);
+  EXPECT_FALSE(tab2->showing_close_button_);
+  EXPECT_FALSE(tab3->showing_close_button_);
 
   // After switching tabs, the previously-active tab should have its
   // tab close button hidden and the newly-active tab should show
@@ -473,26 +464,26 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenStacked) {
   tab_strip_->SelectTab(tab2);
   ASSERT_FALSE(tab1->IsActive());
   ASSERT_TRUE(tab2->IsActive());
-  EXPECT_FALSE(IsTabShowingCloseButton(tab0));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab1));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab2));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab3));
+  EXPECT_FALSE(tab0->showing_close_button_);
+  EXPECT_FALSE(tab1->showing_close_button_);
+  EXPECT_TRUE(tab2->showing_close_button_);
+  EXPECT_FALSE(tab3->showing_close_button_);
 
   // After closing the active tab, the tab which becomes active should
   // show its tab close button.
   tab_strip_->CloseTab(tab1, CLOSE_TAB_FROM_TOUCH);
   tab1 = nullptr;
   ASSERT_TRUE(tab2->IsActive());
-  EXPECT_FALSE(IsTabShowingCloseButton(tab0));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab2));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab3));
+  EXPECT_FALSE(tab0->showing_close_button_);
+  EXPECT_TRUE(tab2->showing_close_button_);
+  EXPECT_FALSE(tab3->showing_close_button_);
 
   // All tab close buttons should be shown when disengaging stacked tab mode.
   tab_strip_->SetStackedLayout(false);
   ASSERT_FALSE(touch_layout());
-  EXPECT_TRUE(IsTabShowingCloseButton(tab0));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab2));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab3));
+  EXPECT_TRUE(tab0->showing_close_button_);
+  EXPECT_TRUE(tab2->showing_close_button_);
+  EXPECT_TRUE(tab3->showing_close_button_);
 }
 
 // Tests that the tab close buttons of non-active tabs are hidden when
@@ -524,9 +515,9 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenNotStacked) {
   ASSERT_FALSE(touch_layout());
 
   // Ensure that all tab close buttons are initially visible.
-  EXPECT_TRUE(IsTabShowingCloseButton(tab0));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab1));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab2));
+  EXPECT_TRUE(tab0->showing_close_button_);
+  EXPECT_TRUE(tab1->showing_close_button_);
+  EXPECT_TRUE(tab2->showing_close_button_);
 
   // Shrink the tab sizes by adding more tabs.
   // An inactive tab added to the tabstrip, now each tab size is not
@@ -534,7 +525,7 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenNotStacked) {
   // tab close button.
   controller_->AddTab(3, false);
   Tab* tab3 = tab_strip_->tab_at(3);
-  EXPECT_FALSE(IsTabShowingCloseButton(tab3));
+  EXPECT_FALSE(tab3->showing_close_button_);
 
   // This inactive tab doesn't have alert button, but its favicon and
   // title would be shown.
@@ -543,18 +534,18 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenNotStacked) {
   EXPECT_TRUE(tab3->title_->visible());
 
   // The active tab's close button still shows.
-  EXPECT_TRUE(IsTabShowingCloseButton(tab1));
+  EXPECT_TRUE(tab1->showing_close_button_);
 
   // An active tab added to the tabstrip should show its tab close
   // button.
   controller_->AddTab(4, true);
   Tab* tab4 = tab_strip_->tab_at(4);
   ASSERT_TRUE(tab4->IsActive());
-  EXPECT_TRUE(IsTabShowingCloseButton(tab4));
+  EXPECT_TRUE(tab4->showing_close_button_);
 
   // The previous active button is now inactive so its close
   // button should not show.
-  EXPECT_FALSE(IsTabShowingCloseButton(tab1));
+  EXPECT_FALSE(tab1->showing_close_button_);
 
   // After switching tabs, the previously-active tab should have its
   // tab close button hidden and the newly-active tab should show
@@ -562,11 +553,11 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenNotStacked) {
   tab_strip_->SelectTab(tab2);
   ASSERT_FALSE(tab4->IsActive());
   ASSERT_TRUE(tab2->IsActive());
-  EXPECT_FALSE(IsTabShowingCloseButton(tab0));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab1));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab2));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab3));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab4));
+  EXPECT_FALSE(tab0->showing_close_button_);
+  EXPECT_FALSE(tab1->showing_close_button_);
+  EXPECT_TRUE(tab2->showing_close_button_);
+  EXPECT_FALSE(tab3->showing_close_button_);
+  EXPECT_FALSE(tab4->showing_close_button_);
 
   // After closing the active tab, the tab which becomes active should
   // show its tab close button.
@@ -574,10 +565,10 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenNotStacked) {
   tab2 = nullptr;
   ASSERT_TRUE(tab3->IsActive());
   DoLayout();
-  EXPECT_FALSE(IsTabShowingCloseButton(tab0));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab1));
-  EXPECT_TRUE(IsTabShowingCloseButton(tab3));
-  EXPECT_FALSE(IsTabShowingCloseButton(tab4));
+  EXPECT_FALSE(tab0->showing_close_button_);
+  EXPECT_FALSE(tab1->showing_close_button_);
+  EXPECT_TRUE(tab3->showing_close_button_);
+  EXPECT_FALSE(tab4->showing_close_button_);
 }
 
 TEST_P(TabStripTest, GetEventHandlerForOverlappingArea) {

chrome/browser/ui/views/tabs/tab_unittest.cc

@@ -719,8 +719,7 @@ TEST_F(TabTest, TitleTextHasSufficientContrast) {
     SkColor fg_active;
     SkColor bg_inactive;
     SkColor fg_inactive;
-  };
-  ColorScheme color_schemes[] = {
+  } color_schemes[] = {
       {
           SK_ColorBLACK, SK_ColorWHITE, SK_ColorBLACK, SK_ColorWHITE,
       },
@@ -731,8 +730,15 @@ TEST_F(TabTest, TitleTextHasSufficientContrast) {
           kDarkGray, kLightGray, kDarkGray, kLightGray,
       },
   };
+
+  // Create a tab inside a Widget, so it has a theme provider, so the call to
+  // UpdateForegroundColors() below doesn't no-op.
+  Widget widget;
+  InitWidget(&widget);
   FakeTabController controller;
   Tab tab(&controller, nullptr);
+  widget.GetContentsView()->AddChildView(&tab);
+
   for (const auto& colors : color_schemes) {
     controller.SetTabColors(colors.bg_active, colors.fg_active,
                             colors.bg_inactive, colors.fg_inactive);