Fix flaky external protocol handler browsertest

Bug: 982234
Change-Id: Ia272a637bd65289092b7ff271b415f750159fd20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1696192
Commit-Queue: Callum May <camay@microsoft.com>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683341}

chrome/browser/external_protocol/external_protocol_handler.cc

@@ -112,7 +112,7 @@ void LaunchUrlWithoutSecurityCheckWithDelegate(
   platform_util::OpenExternal(
       Profile::FromBrowserContext(web_contents->GetBrowserContext()), url);
 
-#if !defined(OS_ANDROID)
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
   // If the protocol navigation occurs in a new tab, close it.
   // Avoid calling CloseContents if the tab is not in this browser's tab strip
   // model; this can happen if the protocol was initiated by something

chrome/browser/external_protocol/external_protocol_handler_browsertest.cc

@@ -2,32 +2,73 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/scoped_observer.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 class ExternalProtocolHandlerBrowserTest : public InProcessBrowserTest {};
 
-const char kScript[] =
-    "new Promise(res => {"
-    "   const tab = window.open('mailto:test@site.test', '_blank');"
-    "   tab.addEventListener('unload', () => {"
-    "     res(true);"
-    "   });"
-    "});";
+// Observe that the tab is created then automatically closed.
+class TabAddedRemovedObserver : public TabStripModelObserver {
+ public:
+  explicit TabAddedRemovedObserver(TabStripModel* tab_strip_model)
+      : scoped_observer_(this) {
+    scoped_observer_.Add(tab_strip_model);
+  }
+
+  void OnTabStripModelChanged(
+      TabStripModel* tab_strip_model,
+      const TabStripModelChange& change,
+      const TabStripSelectionChange& selection) override {
+    if (change.type() == TabStripModelChange::kInserted) {
+      inserted_ = true;
+      return;
+    }
+    if (change.type() == TabStripModelChange::kRemoved) {
+      EXPECT_TRUE(inserted_);
+      removed_ = true;
+      loop_.Quit();
+      return;
+    }
+    NOTREACHED();
+  }
+
+  void Wait() {
+    if (inserted_ && removed_)
+      return;
+    loop_.Run();
+  }
 
-// http://crbug.com/982234
+ private:
+  bool inserted_ = false;
+  bool removed_ = false;
+  base::RunLoop loop_;
+  ScopedObserver<TabStripModel, TabAddedRemovedObserver> scoped_observer_;
+};
+
+IN_PROC_BROWSER_TEST_F(ExternalProtocolHandlerBrowserTest,
+                       AutoCloseTabOnNonWebProtocolNavigation) {
 #if defined(OS_WIN)
-#define MAYBE_AutoCloseTabOnNonWebProtocolNavigation \
-  DISABLED_AutoCloseTabOnNonWebProtocolNavigation
-#else
-#define MAYBE_AutoCloseTabOnNonWebProtocolNavigation \
-  AutoCloseTabOnNonWebProtocolNavigation
+  // On Win 7 the protocol is registered to be handled by Chrome and thus never
+  // reaches the ExternalProtocolHandler so we skip the test. For
+  // more info see installer/util/shell_util.cc:GetShellIntegrationEntries
+  if (base::win::GetVersion() < base::win::Version::WIN8)
+    return;
 #endif
-IN_PROC_BROWSER_TEST_F(ExternalProtocolHandlerBrowserTest,
-                       MAYBE_AutoCloseTabOnNonWebProtocolNavigation) {
+
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
-  EXPECT_EQ(true, EvalJs(web_contents, kScript));
+  TabAddedRemovedObserver observer(browser()->tab_strip_model());
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  ASSERT_TRUE(
+      ExecJs(web_contents, "window.open('mailto:test@site.test', '_blank');"));
+  observer.Wait();
+  EXPECT_EQ(browser()->tab_strip_model()->count(), 1);
 }

chrome/test/BUILD.gn

@@ -1422,7 +1422,7 @@ if (!is_android) {
           [ "../browser/ssl/captive_portal_blocking_page_browsertest.cc" ]
     }
 
-    if (!is_android) {
+    if (!is_android && !is_chromeos) {
       sources += [
         "../browser/external_protocol/external_protocol_handler_browsertest.cc",
       ]