UI test for browser termination with beforeunload handler.

chrome/test/ui/ui_test.cc

 // Copyright (c) 2011 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/test/ui/ui_test.h"
 
 #if defined(OS_POSIX)
 #include <signal.h>
 #include <sys/types.h>
 #endif
@@ skipping 19 matching lines @@
 #include "base/threading/platform_thread.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/common/automation_messages.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/logging_chrome.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/automation/automation_proxy.h"
 #include "chrome/test/automation/browser_proxy.h"
 #include "chrome/test/automation/javascript_execution_controller.h"
 #include "chrome/test/automation/proxy_launcher.h"
 #include "chrome/test/automation/tab_proxy.h"
 #include "chrome/test/automation/window_proxy.h"
 #include "chrome/test/base/chrome_process_util.h"
 #include "chrome/test/base/test_switches.h"
 #include "content/common/debug_flags.h"
@@ skipping 743 matching lines @@
     base::PlatformThread::Sleep(TestTimeouts::action_timeout_ms() / kCycles);
   }
 
   LOG(INFO) << "Elapsed time: " << (base::Time::Now() - start).InSecondsF()
             << " seconds"
             << " call failed " << fail_count << " times"
             << " state was incorrect " << incorrect_state_count << " times";
   ADD_FAILURE() << "Timeout reached in " << __FUNCTION__;
   return false;
 }
+
+void UITest::VerifyCleanExit() {
+  // Make sure the UMA metrics say we didn't crash.

[Paweł Hajdan Jr., 2011/10/24 07:32:51]

How about doing that in TerminateBrowser instead? Every time we shutdown the
browser it should shutdown cleanly.

In fact, I'd like this to be done for most UI tests, unless they say they expect
some crashes or assertions to happen.

[achuithb, 2011/10/24 10:12:39]

Since this hardens the tests, I expect there'll be a few failures that result
from your suggestion, pointing to bugs both in the ui tests and the shutdown
code (from my understanding, both these areas have fragile code).

I'd like to land this CL as is, and then post a separate CL that calls
VerifyCleanExit in TerminateBrowser, to reduce the risk of having this CL
reverted. WDYT?

Please note that this CL is a companion to
http://codereview.chromium.org/8262009/, and we will probably merge these 2 CLs
before landing.

[Paweł Hajdan Jr., 2011/10/24 12:32:14]

Generally sounds good, please add a TODO then. Please note that in my opinion
doing VerifyOnExit in more cases is quite important, so please do that as the
very next thing after landing. Thanks!

[achuithb, 2011/10/24 18:57:19]

Done. I've opened http://crbug.com/101390 to track this.

+  scoped_ptr<DictionaryValue> local_prefs(GetLocalState());
+  bool exited_cleanly;
+  ASSERT_TRUE(local_prefs.get());
+  ASSERT_TRUE(local_prefs->GetBoolean(prefs::kStabilityExitedCleanly,
+                                      &exited_cleanly));
+  ASSERT_TRUE(exited_cleanly);
+
+  // And that session end was successful.
+  bool session_end_completed;
+  ASSERT_TRUE(local_prefs->GetBoolean(prefs::kStabilitySessionEndCompleted,
+                                      &session_end_completed));
+  ASSERT_TRUE(session_end_completed);
+
+  // Make sure session restore says we didn't crash.
+  scoped_ptr<DictionaryValue> profile_prefs(GetDefaultProfilePreferences());
+  ASSERT_TRUE(profile_prefs.get());
+  ASSERT_TRUE(profile_prefs->GetBoolean(prefs::kSessionExitedCleanly,
+                                        &exited_cleanly));
+  ASSERT_TRUE(exited_cleanly);
+}