Fix: tabs slow to response to beforeunload are closed prematurely.

chrome/browser/lifetime/browser_close_manager_browsertest.cc

 // Copyright 2013 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 "chrome/browser/lifetime/browser_close_manager.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 461 matching lines @@
 
   RepeatedNotificationObserver close_observer(
       chrome::NOTIFICATION_BROWSER_CLOSED, 3);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(AcceptClose());
   close_observer.Wait();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(BrowserList::GetInstance()->empty());
 }
 
+// Test that tabs that slow to respond are not closed prematurely.
+IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
+                       TestUnloadMultipleSlowTabs) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  for (int i = 0; i < 7; i++) {

[Charlie Reis, 2016/05/13 22:24:01]

Can you add a comment to this block saying why you're loading one instance of
beforeunload.html vs lots of beforeunload_slow.html?

[Mikhail Goncharov, 2016/05/16 06:10:30]

I have added comment and uploaded next patchset.

+    if (i)
+      AddBlankTabAndShow(browsers_[0]);
+    if (i == 3) {
+      ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
+          browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
+    } else {
+      ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
+          browsers_[0],
+          embedded_test_server()->GetURL("/beforeunload_slow.html")));
+    }
+  }
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
+  chrome::CloseAllBrowsersAndQuit();
+  ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
+  EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
+  // All tabs should still be open.
+  EXPECT_EQ(7, browsers_[0]->tab_strip_model()->count());
+
+  RepeatedNotificationObserver close_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSED, 1);
+  chrome::CloseAllBrowsersAndQuit();
+  ASSERT_NO_FATAL_FAILURE(AcceptClose());
+  close_observer.Wait();
+  EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
+  EXPECT_TRUE(BrowserList::GetInstance()->empty());
+}
+
+// Test that tabs in different windows with a slow beforeunload event response
+// are treated the same as the user accepting the close, but do not close the
+// tab early.
+IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
+                       TestBeforeUnloadMultipleSlowWindows) {

[Charlie Reis, 2016/05/13 22:24:01]

Is there a meaningful difference between slow windows and slow tabs?  I would
imagine that's the same code, but I don't know the UnloadController.

[Mikhail Goncharov, 2016/05/16 04:50:40]

There is a subtle difference on how unload controller iterates over windows vs
tabs and this test fails in a different way with no fix (on (d)check that tab
closed on another tabsstrip)

+  ASSERT_TRUE(embedded_test_server()->Start());
+  for (int i = 0; i < 5; i++) {
+    if (i)
+      browsers_.push_back(CreateBrowser(browser()->profile()));
+    if (i == 2) {
+      ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
+          browsers_[i], embedded_test_server()->GetURL("/beforeunload.html")));
+    } else {
+      ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
+          browsers_[i],
+          embedded_test_server()->GetURL("/beforeunload_slow.html")));
+    }
+  }
+
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 3);
+  chrome::CloseAllBrowsersAndQuit();
+  ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
+  EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
+  // All windows should still be open.
+  for (int i = 0; i < 5; i++)
+    EXPECT_EQ(1, browsers_[i]->tab_strip_model()->count());
+
+  RepeatedNotificationObserver close_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSED, 5);
+  chrome::CloseAllBrowsersAndQuit();
+  ASSERT_NO_FATAL_FAILURE(AcceptClose());
+  close_observer.Wait();
+  EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
+  EXPECT_TRUE(BrowserList::GetInstance()->empty());
+}
+
 // Test that a window created during shutdown is closed.
 IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
                        TestAddWindowDuringShutdown) {
   ASSERT_TRUE(embedded_test_server()->Start());
   ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
       browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
 
   RepeatedNotificationObserver close_observer(
       chrome::NOTIFICATION_BROWSER_CLOSED, 2);
   chrome::CloseAllBrowsersAndQuit();
@@ skipping 541 matching lines @@
 
   chrome::CloseAllBrowsers();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(BrowserList::GetInstance()->empty());
   EXPECT_TRUE(IsBackgroundModeSuspended());
 }
 
 INSTANTIATE_TEST_CASE_P(BrowserCloseManagerWithBackgroundModeBrowserTest,
                         BrowserCloseManagerWithBackgroundModeBrowserTest,
                         testing::Bool());