Refactoring MediaSessionVisibility browser tests

content/browser/media/session/media_session_visibility_browsertest.cc

Index: content/browser/media/session/media_session_visibility_browsertest.cc
diff --git a/content/browser/media/session/media_session_visibility_browsertest.cc b/content/browser/media/session/media_session_visibility_browsertest.cc
index 70ddbfbf4c1ec4b7d81f950601091b2edaac9027..e72d3379978084387b810096874fcfdce5522cfb 100644
--- a/content/browser/media/session/media_session_visibility_browsertest.cc
+++ b/content/browser/media/session/media_session_visibility_browsertest.cc
@@ -24,6 +24,36 @@ static const char kStartPlayerScript[] =
     "document.getElementById('long-video').play()";
 static const char kPausePlayerScript[] =
     "document.getElementById('long-video').pause()";
+
+enum class MediaSuspend {
+  ENABLED,
+  DISABLED,
+};
+
+enum class Pipeline {
+  WMPI,
+  WMPA,
+};
+
+enum class BackgroundResuming {
+  ENABLED,
+  DISABLED,
+};
+
+enum class SessionState {
+  ACTIVE,
+  SUSPENDED,
+  INACTIVE,
+};
+
+struct VisibilityTestData {
+  MediaSuspend media_suspend;
+  Pipeline pipeline;
+  BackgroundResuming background_resuming;
+  SessionState session_state_before_hide;
+  SessionState session_state_after_hide;
+};
+
 }
 
 
@@ -34,7 +64,8 @@ static const char kPausePlayerScript[] =
 // include required tests. See
 // media_session_visibility_browsertest_instances.cc for examples.
 class MediaSessionVisibilityBrowserTest
-    : public ContentBrowserTest {
+    : public ContentBrowserTest,
+      public ::testing::WithParamInterface<VisibilityTestData> {
  public:
   MediaSessionVisibilityBrowserTest() = default;
   ~MediaSessionVisibilityBrowserTest() override = default;
@@ -79,16 +110,83 @@ class MediaSessionVisibilityBrowserTest
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
-    EnableDisableResumingBackgroundVideos(false);
-
     command_line->AppendSwitch(
         switches::kDisableGestureRequirementForMediaPlayback);
 #if !defined(OS_ANDROID)
     command_line->AppendSwitch(
         switches::kEnableDefaultMediaSession);
 #endif  // !defined(OS_ANDROID)
+
+    VisibilityTestData params = GetParam();
+
+    if (params.media_suspend == MediaSuspend::ENABLED)
+      command_line->AppendSwitch(switches::kEnableMediaSuspend);
+    else
+      command_line->AppendSwitch(switches::kDisableMediaSuspend);
+
+#if defined(OS_ANDROID)
+    if (params.pipeline == Pipeline::WMPA)
+      command_line->AppendSwitch(switches::kDisableUnifiedMediaPipeline);
+#endif  // defined(OS_ANDROID)
+
+    if (params.background_resuming == BackgroundResuming::ENABLED) {
+      command_line->AppendSwitchASCII(switches::kEnableFeatures,
+                                      media::kResumeBackgroundVideo.name);
+    } else {
+      command_line->AppendSwitchASCII(switches::kDisableFeatures,
+                                      media::kResumeBackgroundVideo.name);
+    }
+  }
+
+  void StartPlayer() {
+    LoadTestPage();
+
+    LOG(INFO) << "Starting player";
+    ClearMediaSessionStateLoopRunners();
+    RunScript(kStartPlayerScript);
+    LOG(INFO) << "Waiting for session to be active";
+    WaitForMediaSessionState(MediaSession::State::ACTIVE);
+  }
+
+  // Maybe pause the player depending on whether the session state before hide
+  // is SUSPENDED.
+  void MaybePausePlayer() {
+    ASSERT_TRUE(GetParam().session_state_before_hide != SessionState::INACTIVE);
+    if (GetParam().session_state_before_hide == SessionState::ACTIVE)
+      return;
+
+    LOG(INFO) << "Pausing player";
+    ClearMediaSessionStateLoopRunners();
+    RunScript(kPausePlayerScript);
+    LOG(INFO) << "Waiting for session to be suspended";
+    WaitForMediaSessionState(MediaSession::State::SUSPENDED);
+  }
+
+  void HideTab() {
+    LOG(INFO) << "Hiding the tab";
+    ClearMediaSessionStateLoopRunners();
+    web_contents_->WasHidden();
+  }
+
+  void CheckSessionStateAfterHide() {
+    MediaSession::State state_before_hide =
+        ToMediaSessionState(GetParam().session_state_before_hide);
+    MediaSession::State state_after_hide =
+        ToMediaSessionState(GetParam().session_state_after_hide);
+
+    if (state_before_hide == state_after_hide) {
+      LOG(INFO) << "Waiting for 1 second and check session state is unchanged";
+      Wait(base::TimeDelta::FromSeconds(1));
+      ASSERT_EQ(media_session_->audio_focus_state_, state_after_hide);
+    } else {
+      LOG(INFO) << "Waiting for Session to change";
+      WaitForMediaSessionState(state_after_hide);
+    }
+
+    LOG(INFO) << "Test succeeded";
   }
 
+ private:
   void LoadTestPage() {
     TestNavigationObserver navigation_observer(shell()->web_contents(), 1);
     shell()->LoadURL(GetTestUrl("media/session", "media-session.html"));
@@ -123,111 +221,21 @@ class MediaSessionVisibilityBrowserTest
     media_session_state_loop_runners_[state]->Run();
   }
 
- protected:
-  void TestSessionInactiveWhenHiddenAfterContentPause() {
-    LoadTestPage();
-
-    LOG(INFO) << "Starting player";
-    ClearMediaSessionStateLoopRunners();
-    RunScript(kStartPlayerScript);
-    LOG(INFO) << "Waiting for Session to be active";
-    WaitForMediaSessionState(MediaSession::State::ACTIVE);
-
-    LOG(INFO) << "Pausing player";
-    ClearMediaSessionStateLoopRunners();
-    RunScript(kPausePlayerScript);
-    LOG(INFO) << "Waiting for Session to be suspended";
-    WaitForMediaSessionState(MediaSession::State::SUSPENDED);
-
-    LOG(INFO) << "Hiding the tab";
-    ClearMediaSessionStateLoopRunners();
-    web_contents_->WasHidden();
-    LOG(INFO) << "Waiting for Session to be inactive";
-    WaitForMediaSessionState(MediaSession::State::INACTIVE);
-
-    LOG(INFO) << "Test succeeded";
-  }
-
-  void TestSessionInactiveWhenHiddenWhilePlaying() {
-    LoadTestPage();
-
-    LOG(INFO) << "Starting player";
-    ClearMediaSessionStateLoopRunners();
-    RunScript(kStartPlayerScript);
-    LOG(INFO) << "Waiting for Session to be active";
-    WaitForMediaSessionState(MediaSession::State::ACTIVE);
-
-    LOG(INFO) << "Hiding the tab";
-    ClearMediaSessionStateLoopRunners();
-    web_contents_->WasHidden();
-    LOG(INFO) << "Waiting for Session to be inactive";
-    WaitForMediaSessionState(MediaSession::State::INACTIVE);
-
-    LOG(INFO) << "Test succeeded";
-  }
-
-  void TestSessionSuspendedWhenHiddenWhilePlaying() {
-    LoadTestPage();
-
-    LOG(INFO) << "Starting player";
-    ClearMediaSessionStateLoopRunners();
-    RunScript(kStartPlayerScript);
-    LOG(INFO) << "Waiting for Session to be active";
-    WaitForMediaSessionState(MediaSession::State::ACTIVE);
-
-    LOG(INFO) << "Hiding the tab";
-    ClearMediaSessionStateLoopRunners();
-    web_contents_->WasHidden();
-    LOG(INFO) << "Waiting for Session to be suspended";
-    WaitForMediaSessionState(MediaSession::State::SUSPENDED);
-
-    LOG(INFO) << "Test succeeded";
-  }
-
-  void TestSessionSuspendedWhenHiddenAfterContentPause() {
-    LoadTestPage();
-
-    LOG(INFO) << "Starting player";
-    ClearMediaSessionStateLoopRunners();
-    RunScript(kStartPlayerScript);
-    LOG(INFO) << "Waiting for Session to be active";
-    WaitForMediaSessionState(MediaSession::State::ACTIVE);
-
-    LOG(INFO) << "Pausing player";
-    ClearMediaSessionStateLoopRunners();
-    RunScript(kPausePlayerScript);
-    LOG(INFO) << "Waiting for Session to be suspended";
-    WaitForMediaSessionState(MediaSession::State::SUSPENDED);
-
-    LOG(INFO) << "Hiding the tab";
-    // Wait for 1 second and check the MediaSession state.
-    // No better solution till now.
-    web_contents_->WasHidden();
-    Wait(base::TimeDelta::FromSeconds(1));
-    ASSERT_EQ(media_session_->audio_focus_state_,
-              MediaSession::State::SUSPENDED);
-
-    LOG(INFO) << "Test succeeded";
-  }
-
-  void TestSessionActiveWhenHiddenWhilePlaying() {
-    LoadTestPage();
-
-    LOG(INFO) << "Starting player";
-    ClearMediaSessionStateLoopRunners();
-    RunScript(kStartPlayerScript);
-    LOG(INFO) << "Waiting for Session to be active";
-    WaitForMediaSessionState(MediaSession::State::ACTIVE);
-
-    LOG(INFO) << "Hiding the tab";
-    // Wait for 1 second and check the MediaSession state.
-    // No better solution till now.
-    web_contents_->WasHidden();
-    Wait(base::TimeDelta::FromSeconds(1));
-    ASSERT_EQ(media_session_->audio_focus_state_,
-              MediaSession::State::ACTIVE);
-
-    LOG(INFO) << "Test succeeded";
+  MediaSession::State ToMediaSessionState(SessionState state) {
+    switch (state) {
+      case SessionState::ACTIVE:
+        return MediaSession::State::ACTIVE;
+        break;
+      case SessionState::SUSPENDED:
+        return MediaSession::State::SUSPENDED;
+        break;
+      case SessionState::INACTIVE:
+        return MediaSession::State::INACTIVE;
+        break;
+      default:
+        ADD_FAILURE() << "invalid SessionState to convert";
+        return MediaSession::State::INACTIVE;
+    }
   }
 
   WebContents* web_contents_;
@@ -243,114 +251,61 @@ class MediaSessionVisibilityBrowserTest
   std::unique_ptr<base::CallbackList<void(MediaSession::State)>::Subscription>
       media_session_state_callback_subscription_;
 
- private:
   DISALLOW_COPY_AND_ASSIGN(MediaSessionVisibilityBrowserTest);
 };
 
-// Helper macro to include tests from the base class.
-#define INCLUDE_TEST_FROM_BASE_CLASS(test_fixture, test_name)   \
-  IN_PROC_BROWSER_TEST_F(test_fixture, test_name) {             \
-    test_name();                                                \
-  }
-
-///////////////////////////////////////////////////////////////////////////////
-// Configuration instances.
-
-// UnifiedPipeline + SuspendOnHide
-class MediaSessionVisibilityBrowserTest_UnifiedPipeline_SuspendOnHide :
-      public MediaSessionVisibilityBrowserTest {
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    MediaSessionVisibilityBrowserTest::SetUpCommandLine(command_line);
-#if !defined(OS_ANDROID)
-    command_line->AppendSwitch(switches::kEnableMediaSuspend);
-#endif  // defined(OS_ANDROID)
-  }
-};
+namespace {
 
-INCLUDE_TEST_FROM_BASE_CLASS(
-    MediaSessionVisibilityBrowserTest_UnifiedPipeline_SuspendOnHide,
-    TestSessionInactiveWhenHiddenAfterContentPause)
-INCLUDE_TEST_FROM_BASE_CLASS(
-    MediaSessionVisibilityBrowserTest_UnifiedPipeline_SuspendOnHide,
-    TestSessionInactiveWhenHiddenWhilePlaying)
-
-IN_PROC_BROWSER_TEST_F(
-    MediaSessionVisibilityBrowserTest_UnifiedPipeline_SuspendOnHide,
-    TestSessionSuspendedWhenHiddenWhilePlaying) {
-  EnableDisableResumingBackgroundVideos(true);
-  TestSessionSuspendedWhenHiddenWhilePlaying();
-}
+VisibilityTestData kTestParams[] = {
+  { MediaSuspend::ENABLED, Pipeline::WMPI, BackgroundResuming::DISABLED,
+    SessionState::SUSPENDED, SessionState::INACTIVE },
+  { MediaSuspend::ENABLED, Pipeline::WMPI, BackgroundResuming::DISABLED,
+    SessionState::ACTIVE, SessionState::INACTIVE },
+  { MediaSuspend::ENABLED, Pipeline::WMPI, BackgroundResuming::ENABLED,
+    SessionState::ACTIVE, SessionState::SUSPENDED },
+  { MediaSuspend::ENABLED, Pipeline::WMPI, BackgroundResuming::ENABLED,
+    SessionState::SUSPENDED, SessionState::SUSPENDED },
+  { MediaSuspend::DISABLED, Pipeline::WMPI, BackgroundResuming::DISABLED,
+    SessionState::SUSPENDED, SessionState::SUSPENDED },
+  { MediaSuspend::DISABLED, Pipeline::WMPI, BackgroundResuming::DISABLED,
+    SessionState::ACTIVE, SessionState::ACTIVE },
+  { MediaSuspend::DISABLED, Pipeline::WMPI, BackgroundResuming::ENABLED,

[whywhat, 2016/09/06 17:37:29]

What about the same case where the initial state is suspended?

What about the inactive session state?

We could've used the Combine generator if we didn't have to pass the result
state of the test to the test :(

[Zhiqiang Zhang (Slow), 2016/09/07 14:32:04]

Added tests for this
I think the case for inactive state is less useful.

+    SessionState::ACTIVE, SessionState::ACTIVE },
 
-IN_PROC_BROWSER_TEST_F(
-    MediaSessionVisibilityBrowserTest_UnifiedPipeline_SuspendOnHide,
-    TestSessionSuspendedWhenHiddenAfterContentPause) {
-  EnableDisableResumingBackgroundVideos(true);
-  TestSessionSuspendedWhenHiddenAfterContentPause();
-}
-
-// UnifiedPipeline + NosuspendOnHide
-class MediaSessionVisibilityBrowserTest_UnifiedPipeline_NosuspendOnHide :
-      public MediaSessionVisibilityBrowserTest {
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    MediaSessionVisibilityBrowserTest::SetUpCommandLine(command_line);
 #if defined(OS_ANDROID)
-    command_line->AppendSwitch(switches::kDisableMediaSuspend);
+  // The WMPA pipeline tests are flaky. Re-enabling with logging to see what's
+  // happening on the bots. See crbug.com/619096.
+  { MediaSuspend::ENABLED, Pipeline::WMPA, BackgroundResuming::DISABLED,

[whywhat, 2016/09/06 17:37:29]

For WMPA we need to make sure that the same set of tests is run.

I'd still prefer having a second test template that would set the WMPA flag and
call the first test. Then instantiate it with the same list as the non-WMPA
template. Both could be #ifdef-ed with OS_ANDROID.

[Zhiqiang Zhang (Slow), 2016/09/07 14:32:04]

Done by separating out Pipeline from the Params, and use testing::Combine to do
this :)

+    SessionState::SUSPENDED, SessionState::INACTIVE },
+  { MediaSuspend::ENABLED, Pipeline::WMPA, BackgroundResuming::DISABLED,
+    SessionState::ACTIVE, SessionState::INACTIVE },
+  { MediaSuspend::ENABLED, Pipeline::WMPA, BackgroundResuming::ENABLED,
+    SessionState::ACTIVE, SessionState::SUSPENDED },
+  { MediaSuspend::ENABLED, Pipeline::WMPA, BackgroundResuming::ENABLED,
+    SessionState::SUSPENDED, SessionState::SUSPENDED },
+  { MediaSuspend::DISABLED, Pipeline::WMPA, BackgroundResuming::DISABLED,
+    SessionState::SUSPENDED, SessionState::SUSPENDED },
+  { MediaSuspend::DISABLED, Pipeline::WMPA, BackgroundResuming::DISABLED,
+    SessionState::ACTIVE, SessionState::ACTIVE },
+  { MediaSuspend::DISABLED, Pipeline::WMPA, BackgroundResuming::ENABLED,
+    SessionState::ACTIVE, SessionState::ACTIVE },
 #endif  // defined(OS_ANDROID)
-  }
-};
-
-INCLUDE_TEST_FROM_BASE_CLASS(
-    MediaSessionVisibilityBrowserTest_UnifiedPipeline_NosuspendOnHide,
-    TestSessionSuspendedWhenHiddenAfterContentPause)
-INCLUDE_TEST_FROM_BASE_CLASS(
-    MediaSessionVisibilityBrowserTest_UnifiedPipeline_NosuspendOnHide,
-    TestSessionActiveWhenHiddenWhilePlaying)
-
-IN_PROC_BROWSER_TEST_F(
-    MediaSessionVisibilityBrowserTest_UnifiedPipeline_NosuspendOnHide,
-    TestSessionActiveWhenHiddenWhilePlayingWithResume) {
-  EnableDisableResumingBackgroundVideos(true);
-  TestSessionActiveWhenHiddenWhilePlaying();
-}
 
-#if defined(OS_ANDROID)
-// AndroidPipeline + SuspendOnHide
-class MediaSessionVisibilityBrowserTest_AndroidPipeline_SuspendOnHide :
-      public MediaSessionVisibilityBrowserTest {
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    MediaSessionVisibilityBrowserTest::SetUpCommandLine(command_line);
-    command_line->AppendSwitch(switches::kDisableUnifiedMediaPipeline);
-  }
 };
 
-// The following tests are flaky. Re-enabling with logging to see what's
-// happening on the bots. See crbug.com/619096.
-INCLUDE_TEST_FROM_BASE_CLASS(
-    MediaSessionVisibilityBrowserTest_AndroidPipeline_SuspendOnHide,
-    TestSessionInactiveWhenHiddenAfterContentPause)
-INCLUDE_TEST_FROM_BASE_CLASS(
-    MediaSessionVisibilityBrowserTest_AndroidPipeline_SuspendOnHide,
-    TestSessionInactiveWhenHiddenWhilePlaying)
-
-// AndroidPipeline + NosuspendOnHide
-class MediaSessionVisibilityBrowserTest_AndroidPipeline_NosuspendOnHide :
-      public MediaSessionVisibilityBrowserTest {
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    MediaSessionVisibilityBrowserTest::SetUpCommandLine(command_line);
-    command_line->AppendSwitch(switches::kDisableUnifiedMediaPipeline);
-    command_line->AppendSwitch(switches::kDisableMediaSuspend);
-  }
-};
+}  // anonymous namespace
+
+IN_PROC_BROWSER_TEST_P(MediaSessionVisibilityBrowserTest,
+                       TestEntryPoint) {
+  StartPlayer();
+  MaybePausePlayer();
+  HideTab();
+  CheckSessionStateAfterHide();
+}
 
-// The following tests are flaky. Re-enabling with logging to see what's
-// happening on the bots. See crbug.com/619096.
-INCLUDE_TEST_FROM_BASE_CLASS(
-    MediaSessionVisibilityBrowserTest_AndroidPipeline_NosuspendOnHide,
-    TestSessionSuspendedWhenHiddenAfterContentPause)
-INCLUDE_TEST_FROM_BASE_CLASS(
-   MediaSessionVisibilityBrowserTest_AndroidPipeline_NosuspendOnHide,
-   TestSessionActiveWhenHiddenWhilePlaying)
 
-#endif  // defined(OS_ANDROID)
+INSTANTIATE_TEST_CASE_P(MediaSessionVisibilityBrowserTestInstances,
+                        MediaSessionVisibilityBrowserTest,
+                        testing::ValuesIn(kTestParams));
 
 }  // namespace content