From 28de7d94b52d84165475ca2b6d98d556b0feaffb Mon Sep 17 00:00:00 2001
From: "cpu@chromium.org"
 <cpu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 23 Feb 2010 01:43:04 +0000
Subject: Fix for crash on early return from browser_main.cc

BrowserImpl dtor assumes full construction of all sub-objects, this is not true
at least in one case (the try chrome again toast). See bug for more details

- Added a UI test to detect these shenaningans in all platforms
- had to hack ui_tests a bit, I hope is palatable

BUG=34799
TEST= UI test included


Review URL: http://codereview.chromium.org/571017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@39690 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome/browser/browser_main.cc  |  8 +++++---
 chrome/browser/first_run_win.cc |  5 +++++
 chrome/browser/sanity_uitest.cc | 22 +++++++++++++++++++++-
 chrome/test/ui/ui_test.h        |  8 ++++++++
 4 files changed, 39 insertions(+), 4 deletions(-)

(limited to 'chrome')

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 57dd2d9..ccb90c1 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -611,19 +611,21 @@ int BrowserMain(const MainFunctionParams& parameters) {
   gtk_util::SetDefaultWindowIcon();
 #endif
 
-#if defined(OS_WIN)
-  // This is experimental code. See first_run_win.cc for more info.
   std::string try_chrome =
       parsed_command_line.GetSwitchValueASCII(switches::kTryChromeAgain);
   if (!try_chrome.empty()) {
+#if defined(OS_WIN)
     Upgrade::TryResult answer =
         Upgrade::ShowTryChromeDialog(StringToInt(try_chrome));
     if (answer == Upgrade::TD_NOT_NOW)
       return ResultCodes::NORMAL_EXIT_CANCEL;
     if (answer == Upgrade::TD_UNINSTALL_CHROME)
       return ResultCodes::NORMAL_EXIT_EXP2;
+#else
+    // We don't support retention experiments on Mac or Linux.
+    return ResultCodes::NORMAL_EXIT;
+#endif  // defined(OS_WIN)
   }
-#endif  // OS_WIN
 
   BrowserInit browser_init;
 
diff --git a/chrome/browser/first_run_win.cc b/chrome/browser/first_run_win.cc
index 3d2c86a..5a3078f 100644
--- a/chrome/browser/first_run_win.cc
+++ b/chrome/browser/first_run_win.cc
@@ -936,6 +936,11 @@ class TryChromeDialog : public views::ButtonListener,
 }  // namespace
 
 Upgrade::TryResult Upgrade::ShowTryChromeDialog(size_t version) {
+  if (version > 10000) {
+    // This is a test value. We want to make sure we exercise
+    // returning this early. See EarlyReturnTest test harness.
+    return Upgrade::TD_NOT_NOW;
+  }
   TryChromeDialog td;
   return td.ShowModal();
 }
diff --git a/chrome/browser/sanity_uitest.cc b/chrome/browser/sanity_uitest.cc
index 2eaded0..ab347bb 100644
--- a/chrome/browser/sanity_uitest.cc
+++ b/chrome/browser/sanity_uitest.cc
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/platform_thread.h"
+#include "chrome/common/chrome_switches.h"
 
 class GoogleTest : public UITest {
  protected:
@@ -45,5 +46,24 @@ TEST_F(ColumnLayout, Crash) {
   // Make sure the navigation succeeded.
   EXPECT_EQ(page_title, GetActiveTabTitle());
 
-  // UIText will check if this crashed.
+  // UITest will check if this crashed.
+}
+
+// By passing kTryChromeAgain with a magic value > 10000 we cause chrome
+// to exit fairly early. This was the cause of crashes. See bug 34799.
+class EarlyReturnTest : public UITest {
+ public:
+  EarlyReturnTest() {
+    wait_for_initial_loads_ = false;
+    // We don't depend on these timeouts, they are set to the minimum so
+    // the automation server waits the minimun amount possible for the
+    // handshake that will never come.
+    set_command_execution_timeout_ms(1);
+    set_action_timeout_ms(1);
+    launch_arguments_.AppendSwitchWithValue(switches::kTryChromeAgain, "10001");
+  }
+};
+
+TEST_F(EarlyReturnTest, ToastCrasher) {
+  // UITest will check if this crashed.
 }
diff --git a/chrome/test/ui/ui_test.h b/chrome/test/ui/ui_test.h
index 362f7dc..4c8c59f 100644
--- a/chrome/test/ui/ui_test.h
+++ b/chrome/test/ui/ui_test.h
@@ -404,8 +404,16 @@ class UITestBase {
     return command_execution_timeout_ms_;
   }
 
+  void set_command_execution_timeout_ms(int timeout) {
+    command_execution_timeout_ms_ = timeout;
+  }
+
   int action_timeout_ms() const { return action_timeout_ms_; }
 
+  void set_action_timeout_ms(int timeout) {
+    action_timeout_ms_ = timeout;
+  }
+
   int action_max_timeout_ms() const { return action_max_timeout_ms_; }
 
   int sleep_timeout_ms() const { return sleep_timeout_ms_; }
-- 
cgit v1.1