Fixed the audio backgrounding bug

chrome/browser/renderer_host/render_process_host_chrome_browsertest.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 "base/command_line.h"
+#include "base/path_service.h"
 #include "base/process/process.h"
+#include "base/test/test_timeouts.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
+#include "net/base/filename_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+
+#if defined(OS_MACOSX)
+#include "content/browser/mach_broker_mac.h"

[gab, 2015/08/18 21:13:00]

@avi: any opinion on the best way to fix DEPS here (chrome/ can't include from
content/ except for content/public)?

Shall we:

1) make a file specific rule to allow
"content/browser/mach_broker_mac.h" from
"chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc" 

or

2) Move mach_broker_mac.h to content/public?

or 

3) Expose IsProcessBackgroundedHelper() in content/public?

or

4) Your idea?

This is now the only blocker for this CL (and without this include the test must
be if-def'ed out on Mac which would be unfortunate...).

Thanks,
Gab

+#endif
 
 using content::RenderViewHost;
 using content::RenderWidgetHost;
 using content::WebContents;
 
 namespace {
 
 int RenderProcessHostCount() {
   content::RenderProcessHost::iterator hosts =
       content::RenderProcessHost::AllHostsIterator();
@@ skipping 181 matching lines @@
 
 
 class ChromeRenderProcessHostTestWithCommandLine
     : public ChromeRenderProcessHostTest {
  protected:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitchASCII(switches::kRendererProcessLimit, "1");
   }
 };
 
+class ChromeRenderProcessHostBackgroundingTest

[gab, 2015/08/18 21:05:37]

Add a comment describing the properties enabled by this fixture (especially the
implicit things the tests depend on, e.g.: it ends off control to the test with
two tabs open one called "no audio" in foreground and another called "audio" in
background with audio in playing state)

[sebsg, 2015/08/18 21:28:32]

Done.

+    : public ChromeRenderProcessHostTest {
+ public:
+  ChromeRenderProcessHostBackgroundingTest() {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(switches::kProcessPerTab);
+  }
+
+  void SetUpOnMainThread() override {
+    ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+
+    // Set up the server and get the test pages.
+    base::FilePath test_data_dir;
+    ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &test_data_dir));
+    embedded_test_server()->ServeFilesFromDirectory(
+        test_data_dir.AppendASCII("chrome/test/data/"));
+    audio_url_ = embedded_test_server()->GetURL("/extensions/loop_audio.html");
+    no_audio_url_ = embedded_test_server()->GetURL("/title1.html");
+
+    // Open a browser, navigate to the audio page and get its WebContent.
+    ui_test_utils::NavigateToURL(browser(), audio_url_);
+    audio_tab_web_content_ =
+        browser()->tab_strip_model()->GetActiveWebContents();
+
+    // Get the process corresponding to the pages and create a new tab for the
+    // no audio page.
+    audio_process_ = ProcessFromHandle(

[gab, 2015/08/18 21:05:37]

Since you're doing subgroups, feels like getting the |audio_process_| belongs in
the previous subgroup.

Or I could see it belong here if done this way:

// Create a new tab for the no audio page and get the processes corresponding to
each page.
no_audio_process_ = ShowSingletonTab(no_audio_url_);
audio_process_ = ProcessFromHandle(
    audio_tab_web_content_->GetRenderProcessHost()->GetHandle());

(slight preference on the reading flow... i.e. the other way it felt as
completing the previous work for audio and doing bulk work for non-audio
together whereas now I find it feels more like the "processes" subgroup of the
work... yea picky.. sorry!)

[sebsg, 2015/08/18 21:28:33]

Done.

+        audio_tab_web_content_->GetRenderProcessHost()->GetHandle());
+    no_audio_process_ = ShowSingletonTab(no_audio_url_);
+
+    ASSERT_TRUE(no_audio_process_.IsValid());
+    ASSERT_TRUE(audio_process_.IsValid());
+    ASSERT_NE(audio_process_.Pid(), no_audio_process_.Pid());
+  }
+
+ protected:
+  bool IsProcessBackgroundedHelper(const base::Process& process) {
+#if defined(OS_MACOSX)
+    content::MachBroker* broker = content::MachBroker::GetInstance();
+    mach_port_t task_port = broker->TaskForPid(process.Pid());
+    return process.IsProcessBackgrounded(task_port);
+#else
+    return process.IsProcessBackgrounded();
+#endif  // defined(OS_MACOSX)
+  }
+
+  base::Process audio_process_;
+  base::Process no_audio_process_;
+
+  GURL audio_url_;
+  GURL no_audio_url_;
+
+  content::WebContents* audio_tab_web_content_;
+};

[gab, 2015/08/18 21:05:37]

 private:
  DISALLOW_COPY_AND_ASSIGN(ChromeRenderProcessHostBackgroundingTest);

(we do this on all classes -- old tests often omitted it, but we try to do it
everywhere now)

[sebsg, 2015/08/18 21:28:32]

Done.

+
 // Disable on Mac and Windows due to ongoing flakiness. (crbug.com/442785)
 #if defined(OS_MACOSX) || defined(OS_WIN)
 #define MAYBE_ProcessPerTab DISABLED_ProcessPerTab
 #else
 #define MAYBE_ProcessPerTab ProcessPerTab
 #endif
 
 IN_PROC_BROWSER_TEST_F(ChromeRenderProcessHostTest, MAYBE_ProcessPerTab) {
   // Set max renderers to 1 to force running out of processes.
   content::RenderProcessHost::SetMaxRendererProcessCount(1);
@@ skipping 287 matching lines @@
       chrome::NOTIFICATION_BROWSER_CLOSED,
       content::NotificationService::AllSources());
 
   // Kill the renderer process, simulating a crash. This should the ProcessDied
   // method to be called. Alternatively, RenderProcessHost::OnChannelError can
   // be called to directly force a call to ProcessDied.
   wc1->GetRenderProcessHost()->Shutdown(-1, true);
 
   observer.Wait();
 }
+

[gab, 2015/08/18 21:05:37]

Move test fixture here as it's not used by tests above (tests tend to keep
things closely related together rather than follow the typical cc file ordering
of declarations...).

[sebsg, 2015/08/18 21:28:32]

Done.

+// Test to make sure that a process is backgrounded when the audio stops playing
+// from the active tab and there is an immediate tab switch.
+IN_PROC_BROWSER_TEST_F(ChromeRenderProcessHostBackgroundingTest,
+                       ProcessPriorityAfterStoppedAudio) {
+  // This test is invalid on platforms that can't background.
+  if (!base::Process::CanBackgroundProcesses())
+    return;
+
+  ShowSingletonTab(audio_url_);
+
+  // Wait until the no audio page is backgrounded and the audio page is not
+  // backgrounded.
+  while (!IsProcessBackgroundedHelper(no_audio_process_) ||
+         IsProcessBackgroundedHelper(audio_process_)) {
+    base::RunLoop().RunUntilIdle();
+    base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  }
+
+  // Pause the audio and immediately switch to the no audio tab.
+  ASSERT_TRUE(content::ExecuteScript(
+      audio_tab_web_content_,
+      "document.getElementById('audioPlayer').pause();"));
+  ShowSingletonTab(no_audio_url_);
+
+  // Wait until the no audio page is not backgrounded and the audio page is
+  // backgrounded.
+  while (IsProcessBackgroundedHelper(no_audio_process_) ||
+         !IsProcessBackgroundedHelper(audio_process_)) {
+    base::RunLoop().RunUntilIdle();
+    base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  }
+}
+
+// Test to make sure that a process is backgrounded automatically when audio
+// stops playing from a not visible tab.

[gab, 2015/08/18 21:05:37]

s/not visible/hidden

[sebsg, 2015/08/18 21:28:33]

Done.

+IN_PROC_BROWSER_TEST_F(ChromeRenderProcessHostBackgroundingTest,
+                       ProcessPriorityAfterAudioStopsOnNotVisibleTab) {
+  // This test is invalid on platforms that can't background.
+  if (!base::Process::CanBackgroundProcesses())
+    return;
+
+  // Wait until the two pages are not backgrounded.

[gab, 2015/08/18 21:05:37]

This assumes things about the test setup, see comment above about making sure
this is obvious from the contract of the fixture.

[sebsg, 2015/08/18 21:28:33]

Done.

+  while (IsProcessBackgroundedHelper(no_audio_process_) ||
+         IsProcessBackgroundedHelper(audio_process_)) {
+    base::RunLoop().RunUntilIdle();
+    base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  }
+
+  // Stop the audio.
+  ASSERT_TRUE(content::ExecuteScript(
+      audio_tab_web_content_,
+      "document.getElementById('audioPlayer').pause();"));
+
+  // Wait until the no audio page is not backgrounded and the audio page is
+  // backgrounded.
+  while (IsProcessBackgroundedHelper(no_audio_process_) ||
+         !IsProcessBackgroundedHelper(audio_process_)) {
+    base::RunLoop().RunUntilIdle();
+    base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  }
+}
+
+// Test to make sure that a process is un-backgrounded automatically when audio
+// starts playing from a backgrounded tab.
+IN_PROC_BROWSER_TEST_F(ChromeRenderProcessHostBackgroundingTest,
+                       ProcessPriorityAfterAudioStartsFromBackgroundTab) {
+  // This test is invalid on platforms that can't background.
+  if (!base::Process::CanBackgroundProcesses())
+    return;
+
+  // Stop the audio.
+  ASSERT_TRUE(content::ExecuteScript(
+      audio_tab_web_content_,
+      "document.getElementById('audioPlayer').pause();"));
+
+  // Wait until the no audio page is not backgrounded and the audio page is
+  // backgrounded.
+  while (IsProcessBackgroundedHelper(no_audio_process_) ||
+         !IsProcessBackgroundedHelper(audio_process_)) {
+    base::RunLoop().RunUntilIdle();
+    base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  }
+
+  // Start the audio from the backgrounded tab.
+  ASSERT_TRUE(
+      content::ExecuteScript(audio_tab_web_content_,
+                             "document.getElementById('audioPlayer').play();"));
+
+  // Wait until the two pages are not backgrounded.
+  while (IsProcessBackgroundedHelper(no_audio_process_) ||
+         IsProcessBackgroundedHelper(audio_process_)) {
+    base::RunLoop().RunUntilIdle();
+    base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  }
+}