Fix TabCapture extension API tests

Currently, some of the TabCapture tests are disabled, due to historic
flakiness. A lot of this is due to using an invalid URL, causing the
web contents to be flaky/broken.

This patch re-enables tests and fixes some of them by using actual
domains and improving test isolation. Some tests are reenabled with the
hope that they will be more more stable now... will have to monitor.

Bug: 427730, 674517, 764464
Change-Id: Ic8cb93388572ff5e5828e6179cb728084c9324d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438290
Commit-Queue: Jordan Bayles <jophba@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812948}

chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc

@@ -31,12 +31,14 @@
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "url/url_constants.h"
 
 namespace extensions {
 
 namespace {
 
-const char kExtensionId[] = "ddchlicdkolnonkihahngkmmmjnjlkkf";
+constexpr char kExtensionId[] = "ddchlicdkolnonkihahngkmmmjnjlkkf";
+constexpr char kValidChromeURL[] = "chrome://version";
 
 class TabCaptureApiTest : public ExtensionApiTest {
  public:
@@ -71,13 +73,8 @@ class TabCaptureApiPixelTest : public TabCaptureApiTest {
 
  protected:
   bool IsTooIntensiveForThisPlatform() const {
-#if defined(NDEBUG)
-    // The tests are too slow to succeed with software GL on the bots.
-    return UsingSoftwareGL();
-#else
-    // The tests only run on release builds.
+    // Timeouts on most bots. crbug.com/864250, crbug.com/1040894
     return true;
-#endif
   }
 };
 
@@ -150,38 +147,22 @@ TEST(TabCaptureCaptureOffscreenTabTest, DetermineInitialSize) {
                 options));
 }
 
-// Flaky on Mac. See https://crbug.com/764464.
-#if defined(OS_MAC) || \
-    ((defined(OS_LINUX) || defined(OS_CHROMEOS)) && defined(MEMORY_SANITIZER))
-#define MAYBE_ApiTests DISABLED_ApiTests
-#else
-#define MAYBE_ApiTests ApiTests
-#endif
 // Tests API behaviors, including info queries, and constraints violations.
-IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, MAYBE_ApiTests) {
+IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, ApiTests) {
   AddExtensionToCommandLineAllowlist();
   ASSERT_TRUE(RunExtensionSubtest("tab_capture", "api_tests.html")) << message_;
 }
 
-#if (defined(OS_MAC) && defined(ADDRESS_SANITIZER)) || defined(OS_LINUX) || \
-    defined(OS_WIN)
-// Flaky on ASAN on Mac, and on Linux and Windows. See https://crbug.com/674497.
-#define MAYBE_MaxOffscreenTabs DISABLED_MaxOffscreenTabs
-#else
-#define MAYBE_MaxOffscreenTabs MaxOffscreenTabs
-#endif
 // Tests that there is a maximum limitation to the number of simultaneous
 // off-screen tabs.
-IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, MAYBE_MaxOffscreenTabs) {
+IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, MaxOffscreenTabs) {
   AddExtensionToCommandLineAllowlist();
   ASSERT_TRUE(RunExtensionSubtest("tab_capture", "max_offscreen_tabs.html"))
       << message_;
 }
 
 // Tests that tab capture video frames can be received in a VIDEO element.
-// Flaky on all platforms. See https://crbug.com/1040894
-IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest,
-                       DISABLED_EndToEndWithoutRemoting) {
+IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest, EndToEndWithoutRemoting) {
   if (IsTooIntensiveForThisPlatform()) {
     LOG(WARNING) << "Skipping this CPU-intensive test on this platform/build.";
     return;
@@ -201,8 +182,7 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest,
 // received in a VIDEO element.  More allowance is provided for color deviation
 // because of the additional layers of video processing performed within
 // WebRTC.
-// Flaky on all platforms. See https://crbug.com/1040894
-IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest, DISABLED_EndToEndThroughWebRTC) {
+IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest, EndToEndThroughWebRTC) {
   if (IsTooIntensiveForThisPlatform()) {
     LOG(WARNING) << "Skipping this CPU-intensive test on this platform/build.";
     return;
@@ -229,18 +209,8 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest, OffscreenTabEndToEnd) {
   ASSERT_FALSE(profile()->HasPrimaryOTRProfile());
 }
 
-#if defined(OS_MAC)
-// Timeout on Mac. crbug.com/864250
-#define MAYBE_OffscreenTabEvilTests DISABLED_OffscreenTabEvilTests
-#elif defined(OS_LINUX) || defined(OS_CHROMEOS)
-// Flaky on Linux and ChromeOS. crbug.com/895120
-#define MAYBE_OffscreenTabEvilTests DISABLED_OffscreenTabEvilTests
-#else
-#define MAYBE_OffscreenTabEvilTests OffscreenTabEvilTests
-#endif
-
 // Tests that off-screen tabs can't do evil things (e.g., access local files).
-IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest, MAYBE_OffscreenTabEvilTests) {
+IN_PROC_BROWSER_TEST_F(TabCaptureApiPixelTest, OffscreenTabEvilTests) {
   if (IsTooIntensiveForThisPlatform()) {
     LOG(WARNING) << "Skipping this CPU-intensive test on this platform/build.";
     return;
@@ -261,7 +231,7 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, GetUserMediaTest) {
 
   EXPECT_TRUE(listener.WaitUntilSatisfied());
 
-  content::OpenURLParams params(GURL("about:blank"), content::Referrer(),
+  content::OpenURLParams params(GURL(url::kAboutBlankURL), content::Referrer(),
                                 WindowOpenDisposition::NEW_FOREGROUND_TAB,
                                 ui::PAGE_TRANSITION_LINK, false);
   content::WebContents* web_contents = browser()->OpenURL(params);
@@ -277,10 +247,9 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, GetUserMediaTest) {
   EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
-// http://crbug.com/427730
 // Make sure tabCapture.capture only works if the tab has been granted
 // permission via an extension icon click or the extension is allowlisted.
-IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, DISABLED_ActiveTabPermission) {
+IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, ActiveTabPermission) {
   ExtensionTestMessageListener before_open_tab("ready1", true);
   ExtensionTestMessageListener before_grant_permission("ready2", true);
   ExtensionTestMessageListener before_open_new_tab("ready3", true);
@@ -292,10 +261,11 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, DISABLED_ActiveTabPermission) {
 
   // Open a new tab and make sure capture is denied.
   EXPECT_TRUE(before_open_tab.WaitUntilSatisfied());
-  content::OpenURLParams params(GURL("http://google.com"), content::Referrer(),
+  content::OpenURLParams params(GURL(url::kAboutBlankURL), content::Referrer(),
                                 WindowOpenDisposition::NEW_FOREGROUND_TAB,
                                 ui::PAGE_TRANSITION_LINK, false);
   content::WebContents* web_contents = browser()->OpenURL(params);
+  ASSERT_TRUE(web_contents) << "Failed to open new tab";
   before_open_tab.Reply("");
 
   // Grant permission and make sure capture succeeds.
@@ -350,10 +320,8 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, FullscreenEvents) {
   EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
-// Flaky on MSan bots: https://crbug.com/294431
-// But really, just flaky everywhere. http://crbug.com/294431#c33
 // Make sure tabCapture API can be granted for Chrome:// pages.
-IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, DISABLED_GrantForChromePages) {
+IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, GrantForChromePages) {
   ExtensionTestMessageListener before_open_tab("ready1", true);
   ASSERT_TRUE(RunExtensionSubtest("tab_capture",
                                   "active_tab_chrome_pages.html"))
@@ -361,7 +329,7 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, DISABLED_GrantForChromePages) {
   EXPECT_TRUE(before_open_tab.WaitUntilSatisfied());
 
   // Open a tab on a chrome:// page and make sure we can capture.
-  content::OpenURLParams params(GURL("chrome://version"), content::Referrer(),
+  content::OpenURLParams params(GURL(kValidChromeURL), content::Referrer(),
                                 WindowOpenDisposition::NEW_FOREGROUND_TAB,
                                 ui::PAGE_TRANSITION_LINK, false);
   content::WebContents* web_contents = browser()->OpenURL(params);

chrome/test/data/extensions/api_test/tab_capture/active_tab_permission_test.js

@@ -8,6 +8,7 @@ var expectedLastErrorMessage =
 
 var afterWhitelistExtension = function(msg) {
   chrome.tabCapture.capture({audio: true, video: true}, function(stream) {
+    chrome.test.assertNoLastError();
     chrome.test.assertTrue(!!stream);
     stream.getVideoTracks()[0].stop();
     stream.getAudioTracks()[0].stop();
@@ -25,6 +26,7 @@ var afterOpenNewTab = function(msg) {
 
 var afterGrantPermission = function(msg) {
   chrome.tabCapture.capture({audio: true, video: true}, function(stream) {
+    chrome.test.assertNoLastError();
     chrome.test.assertTrue(!!stream);
     stream.getVideoTracks()[0].stop();
     stream.getAudioTracks()[0].stop();

chrome/test/data/extensions/api_test/tab_capture/api_tests.js

@@ -19,6 +19,23 @@ function assertIsSameSetOfTabs(list_a, list_b, id_field_name) {
   }
 }
 
+// We can't actually unsubscribe from onStatusChanged, so make sure it
+// only fires once per test.
+let g_test_index = 0;
+function testSucceedOnCaptureEnd() {
+  g_test_index += 1;
+  const my_test_index = g_test_index;
+  var tabCaptureListener = function(info) {
+    console.log(
+        my_test_index + ' tabCapture.onStatusChanged to: ' + info.status);
+    if (info.status == 'stopped' && my_test_index == g_test_index) {
+      chrome.test.succeed();
+      return;
+    }
+  };
+  tabCapture.onStatusChanged.addListener(tabCaptureListener);
+}
+
 function assertIsValidStreamId(streamId) {
   chrome.test.assertTrue(typeof streamId == 'string');
   navigator.webkitGetUserMedia({
@@ -31,8 +48,8 @@ function assertIsValidStreamId(streamId) {
     }
   }, function(stream) {
     chrome.test.assertTrue(!!stream);
+    testSucceedOnCaptureEnd();
     stream.getVideoTracks()[0].stop();
-    chrome.test.succeed();
   }, function(error) {
     chrome.test.fail(error);
   });
@@ -156,16 +173,16 @@ var testsToRun = [
   function onlyVideo() {
     tabCapture.capture({video: true}, function(stream) {
       chrome.test.assertTrue(!!stream);
+      testSucceedOnCaptureEnd();
       stream.getVideoTracks()[0].stop();
-      chrome.test.succeed();
     });
   },
 
   function onlyAudio() {
     tabCapture.capture({audio: true}, function(stream) {
       chrome.test.assertTrue(!!stream);
+      testSucceedOnCaptureEnd();
       stream.getAudioTracks()[0].stop();
-      chrome.test.succeed();
     });
   },
 

chrome/test/data/extensions/api_test/tab_capture/max_offscreen_tabs.js

@@ -4,14 +4,25 @@
 
 var tabCapture = chrome.tabCapture;
 
-var helloWorldPageUri = 'data:text/html;charset=UTF-8,' +
-    encodeURIComponent('<html><body>Hello world!</body></html>');
+const helloWorldPageUri =
+    'data:text/html;charset=UTF-8,' +
+    encodeURIComponent(
+        '<html><body>Hello world!</body></html>' +
+        '<script>setTimeout(() => { window.close(); }, 3000);</script>');
 
 chrome.test.runTests([
  function canOpenUpToThreeOffscreenTabs() {
    function stopAllStreams(streams) {
+     // Off-screen tabs remain alive until either all MediaStreams are closed,
+     // the page self-closes, or the extension that called captureOffscreenTab
+     // is unloaded.
      for (var i = 0, end = streams.length; i < end; ++i) {
        streams[i].getVideoTracks()[0].stop();
+
+       audio_track = streams[i].getAudioTracks()[0];
+       if (audio_track) {
+         audio_track.stop();
+       }
      }
    }