Set Ash material design mode in tests properly

ash/shelf/shelf_view_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/shelf/shelf_view.h"
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 #include <vector>
@@ skipping 2098 matching lines @@
   }
 
  private:
   std::unique_ptr<TestAppListPresenter> app_list_presenter_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAppListShellDelegate);
 };
 
 }  // namespace
 
 class ShelfViewInkDropTest : public ShelfViewTest {

[tdanderson, 2016/07/29 15:16:50]

Can you add a comment here saying that this fixture forces MD to be enabled for
the test run (i.e., test fixture is only applicable with the MD feature work)

[bruthig, 2016/07/29 16:13:03]

Are there any tests here that are NON_MATERIAL specific, or should there be? 
i.e. Should we be running these tests with more than just the one material
design mode?

[mohsen, 2016/07/29 18:05:19]

Done.

[mohsen, 2016/07/29 18:05:19]

No, these are all ripple tests and only make sense in material mode.

  public:
   ShelfViewInkDropTest() {}
   ~ShelfViewInkDropTest() override {}
 
   void SetUp() override {
     shell_delegate_ = new TestAppListShellDelegate;
     ash_test_helper()->set_test_shell_delegate(shell_delegate_);
 
+    set_material_mode(ash::MaterialDesignController::MATERIAL_EXPERIMENTAL);

[tdanderson, 2016/07/29 15:16:50]

Just a thought (not necessarily something to address in this CL): if there are
going to be lots of places where we will need to make calls like this, maybe we
could change MaterialDesignController to look something like:

Mode MDC::ModeForShelf() {
  return EXPERIMENTAL;  // (*)
}

bool MDC::IsShelfMaterial() {
  return mode() == ModeForShelf();
}

then line 2128 could be:

set_material_mode(MDC::ModeForShelf());

The advantage is that when the MD shelf needs to change mode (e.g., if we
promote from EXPERIMENTAL to NORMAL) then (*) would be the only line that needs
to change.

[bruthig, 2016/07/29 16:13:04]

+1 to something like this. Could "ModeForShelf()" be replaced with a constant?

[mohsen, 2016/07/29 18:05:19]

As discussed in person, this is not simple since EXPERIMENTAL also implies
NORMAL. For example, when shelf is promoted to NORMAL, then IsShelfMaterial()
should return true not just for NORMAL but also for EXPERIMENTAL. Maybe we need
to add some sort of ordering between modes. Anyways, better to address in
anotehr CL.

+
     ShelfViewTest::SetUp();
-
-    // TODO(mohsen): Ideally, we would want to set material mode before calling
-    // ShelfViewTest::SetUp() so that everything is set up with the correct
-    // material mode. Currently, this is not possible as it expects material
-    // mode be UNINITIALIZED. (See https://crbug.com/620093)
-    ash_md_controller_.reset(new ash::test::MaterialDesignControllerTestAPI(
-        ash::MaterialDesignController::MATERIAL_EXPERIMENTAL));
-  }
-
-  void TearDown() override {
-    ash_md_controller_.reset();
-
-    ShelfViewTest::TearDown();
   }
 
  protected:
   void InitAppListButtonInkDrop() {
     app_list_button_ = shelf_view_->GetAppListButton();
 
     views::InkDropImpl* ink_drop_impl =
         new views::InkDropImpl(app_list_button_);
     app_list_button_ink_drop_ = new InkDropSpy(base::WrapUnique(ink_drop_impl));
     views::test::InkDropHostViewTestApi(app_list_button_)
@@ skipping 20 matching lines @@
     shell_delegate_->app_list_presenter()->Dismiss();
     // Similar to real AppListPresenter, notify button that the app list is
     // dismissed.
     app_list_button_->OnAppListDismissed();
   }
 
   void FinishAppListVisibilityChange() {
     shell_delegate_->app_list_presenter()->FinishVisibilityChange();
   }
 
-  std::unique_ptr<ash::test::MaterialDesignControllerTestAPI>
-      ash_md_controller_;
-
   TestAppListShellDelegate* shell_delegate_ = nullptr;  // Owned by Shell.
 
   AppListButton* app_list_button_ = nullptr;
   InkDropSpy* app_list_button_ink_drop_ = nullptr;
   ShelfButton* browser_button_ = nullptr;
   InkDropSpy* browser_button_ink_drop_ = nullptr;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ShelfViewInkDropTest);
 };
@@ skipping 380 matching lines @@
   test_api_->CloseMenu();
   EXPECT_EQ(views::InkDropState::HIDDEN,
             browser_button_ink_drop_->GetTargetInkDropState());
   EXPECT_THAT(browser_button_ink_drop_->GetAndResetRequestedStates(),
               ElementsAre(views::InkDropState::ACTIVATED,
                           views::InkDropState::DEACTIVATED));
 }
 
 }  // namespace test
 }  // namespace ash