Fix TabStripTest when GetParam returns true.

In an unrelated patch I discovered that there was no difference between
the two Param variables in TabStripTest. is_touch was unexpectedly
still false inside GetLayoutConstants even though it should have been
overridden to true by TabStripTest::test_api_ .

Because ViewsTestBase was calling MaterialDesignController::Initialize()
inside SetUp, the value set by TabStripTest::test_api_ was being
overwritten. To fix this, the ViewsTestBase call is moved to its
constructor, so that it will always happen first.

This caused TabCloseButtonVisibilityWhenNotStacked/1 to fail, which was
because FakeBaseTabStripController was calling TabStrip::SetSelection
before TabStrip::RemoveTabAt, which seems wrong (the tab at
active_index_ isn't removed yet). In non-test code, TabStripModel will
call RemoveTabAt first, so this patch switches the order and fixes the
test. It is unknown why TabCloseButtonVisibilityWhenNotStacked/0 was
not failing.

This patch also adds a test to ensure the layout is different when
switching from touch UI to normal UI and vice versa.

Change-Id: I447ffe0ebd99cb56ab77c23e60fa67cf09ba98f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1747175Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686515}

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

@@ -51,12 +51,15 @@ void FakeBaseTabStripController::RemoveTab(int index) {
   num_tabs_--;
   // RemoveTabAt() expects the controller state to have been updated already.
   const bool was_active = index == active_index_;
-  if (active_index_ > index) {
+  if (was_active) {
+    active_index_ = std::min(active_index_, num_tabs_ - 1);
+    selection_model_.SetSelectedIndex(active_index_);
+  } else if (active_index_ > index) {
     --active_index_;
-  } else if (active_index_ == index) {
-    SetActiveIndex(std::min(active_index_, num_tabs_ - 1));
   }
   tab_strip_->RemoveTabAt(nullptr, index, was_active);
+  if (was_active && IsValidIndex(active_index_))
+    tab_strip_->SetSelection(selection_model_);
 }
 
 void FakeBaseTabStripController::MoveTabIntoGroup(

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

@@ -1128,4 +1128,23 @@ TEST_P(TabStripTest, DeleteTabGroupHeaderWhenEmpty) {
   EXPECT_EQ(0u, ListGroupHeaders().size());
 }
 
+TEST_P(TabStripTest, ChangingLayoutTypeResizesTabs) {
+  tab_strip_->SetBounds(0, 0, 1000, 100);
+
+  tab_strip_->AddTabAt(0, TabRendererData(), false);
+  Tab* tab = tab_strip_->tab_at(0);
+  const int initial_height = tab->height();
+
+  ui::test::MaterialDesignControllerTestAPI other_layout(!GetParam());
+
+  CompleteAnimationAndLayout();
+  if (GetParam()) {
+    // Touch -> normal.
+    EXPECT_LT(tab->height(), initial_height);
+  } else {
+    // Normal -> touch.
+    EXPECT_GT(tab->height(), initial_height);
+  }
+}
+
 INSTANTIATE_TEST_SUITE_P(, TabStripTest, ::testing::Values(false, true));

ui/views/test/views_test_base.cc

@@ -64,7 +64,12 @@ bool InitializeVisuals() {
 
 }  // namespace
 
-ViewsTestBase::ViewsTestBase() = default;
+ViewsTestBase::ViewsTestBase() {
+  // MaterialDesignController is initialized here instead of in SetUp because
+  // a subclass might construct a MaterialDesignControllerTestAPI as a member to
+  // override the value, and this must happen first.
+  ui::MaterialDesignController::Initialize();
+}
 
 ViewsTestBase::~ViewsTestBase() {
   CHECK(setup_called_)
@@ -82,7 +87,6 @@ void ViewsTestBase::SetUp() {
   has_compositing_manager_ = InitializeVisuals();
 
   testing::Test::SetUp();
-  ui::MaterialDesignController::Initialize();
   setup_called_ = true;
   if (!views_delegate_for_setup_)
     views_delegate_for_setup_ = std::make_unique<TestViewsDelegate>();