Re-apply http://codereview.chromium.org/1622012

It was reverted as http://codereview.chromium.org/1822001 the fix in Valgrind scripts has been commited as http://codereview.chromium.org/1736026
TEST=trybots

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46070 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 142 insertions, 94 deletions

diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 9618418..47b1438 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -519,7 +519,7 @@ void ProfileSyncService::ChangePreferredDataTypes(
 
 void ProfileSyncService::GetPreferredDataTypes(
     syncable::ModelTypeSet* preferred_types) const {
- preferred_types->clear();
+  preferred_types->clear();
 
   // Filter out any datatypes which aren't registered, or for which
   // the preference can't be read.
diff --git a/chrome/browser/sync/protocol/sync_proto.gyp b/chrome/browser/sync/protocol/sync_proto.gyp
new file mode 100755
index 0000000..4628ab6
--- /dev/null
+++ b/chrome/browser/sync/protocol/sync_proto.gyp
@@ -0,0 +1,73 @@
+# Copyright (c) 2010 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.
+
+{
+  'variables': {
+    'chromium_code': 1,
+  },
+  'targets': [
+    {
+      # Protobuf compiler / generate rule for sync.proto.  This is used by
+      # test code in net, which is why it's isolated into its own .gyp file.
+      'target_name': 'sync_proto',
+      'type': 'none',
+      'sources': [
+        'sync.proto',
+        'autofill_specifics.proto',
+        'bookmark_specifics.proto',
+        'preference_specifics.proto',
+        'theme_specifics.proto',
+        'typed_url_specifics.proto',
+      ],
+      'rules': [
+        {
+          'rule_name': 'genproto',
+          'extension': 'proto',
+          'inputs': [
+            '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
+          ],
+          'outputs': [
+            '<(PRODUCT_DIR)/pyproto/sync_pb/<(RULE_INPUT_ROOT)_pb2.py',
+            '<(SHARED_INTERMEDIATE_DIR)/protoc_out/chrome/browser/sync/protocol/<(RULE_INPUT_ROOT).pb.h',
+            '<(SHARED_INTERMEDIATE_DIR)/protoc_out/chrome/browser/sync/protocol/<(RULE_INPUT_ROOT).pb.cc',
+          ],
+          'action': [
+            '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
+            '--proto_path=.',
+            './<(RULE_INPUT_ROOT)<(RULE_INPUT_EXT)',
+            '--cpp_out=<(SHARED_INTERMEDIATE_DIR)/protoc_out/chrome/browser/sync/protocol',
+            '--python_out=<(PRODUCT_DIR)/pyproto/sync_pb',
+          ],
+          'message': 'Generating C++ and Python code from <(RULE_INPUT_PATH)',
+        },
+      ],
+      'dependencies': [
+        '../../../../third_party/protobuf2/protobuf.gyp:protoc#host',
+      ],
+    },
+    {
+      'target_name': 'sync_proto_cpp',
+      'type': 'none',
+      'export_dependent_settings': [
+        '../../../../third_party/protobuf2/protobuf.gyp:protobuf_lite',
+        'sync_proto',
+      ],
+      'dependencies': [
+        '../../../../third_party/protobuf2/protobuf.gyp:protobuf_lite',
+        'sync_proto',
+      ],
+      'direct_dependent_settings': {
+        'include_dirs': [
+          '<(SHARED_INTERMEDIATE_DIR)/protoc_out',
+        ],
+      },
+    },
+  ],
+}
+
+# Local Variables:
+# tab-width:2
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=2 shiftwidth=2:
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 35d7319..86af93c 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -689,61 +689,10 @@
         '../third_party/icu/icu.gyp:icuuc',
         '../third_party/libjingle/libjingle.gyp:libjingle',
         '../third_party/sqlite/sqlite.gyp:sqlite',
+        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'common_constants',
         'notifier',
         'sync',
-        'sync_proto',
-      ],
-    },
-    {
-      # Protobuf compiler / generate rule for sync.proto
-      'target_name': 'sync_proto',
-      'type': 'none',
-      'sources': [
-        'browser/sync/protocol/sync.proto',
-        'browser/sync/protocol/autofill_specifics.proto',
-        'browser/sync/protocol/bookmark_specifics.proto',
-        'browser/sync/protocol/preference_specifics.proto',
-        'browser/sync/protocol/theme_specifics.proto',
-        'browser/sync/protocol/typed_url_specifics.proto',
-      ],
-      'rules': [
-        {
-          'rule_name': 'genproto',
-          'extension': 'proto',
-          'inputs': [
-            '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
-          ],
-          'variables': {
-            # The protoc compiler requires a proto_path argument with the
-            # directory containing the .proto file.
-            # There's no generator variable that corresponds to this, so fake it.
-            'rule_input_relpath': 'browser/sync/protocol',
-          },
-          'outputs': [
-            '<(protoc_out_dir)/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT).pb.h',
-            '<(protoc_out_dir)/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT).pb.cc',
-          ],
-          'action': [
-            '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
-            '--proto_path=./<(rule_input_relpath)',
-            './<(rule_input_relpath)/<(RULE_INPUT_ROOT)<(RULE_INPUT_EXT)',
-            '--cpp_out=<(protoc_out_dir)/chrome/<(rule_input_relpath)',
-          ],
-          'message': 'Generating C++ code from <(RULE_INPUT_PATH)',
-        },
-      ],
-      'dependencies': [
-        '../third_party/protobuf2/protobuf.gyp:protobuf_lite',
-        '../third_party/protobuf2/protobuf.gyp:protoc#host',
-      ],
-      'direct_dependent_settings': {
-        'include_dirs': [
-          '<(protoc_out_dir)',
-        ]
-      },
-      'export_dependent_settings': [
-        '../third_party/protobuf2/protobuf.gyp:protobuf_lite',
       ],
     },
     {
@@ -836,7 +785,7 @@
         '../net/net.gyp:net',
         '../third_party/expat/expat.gyp:expat',
         '../third_party/libjingle/libjingle.gyp:libjingle',
-        'sync_proto',
+        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
       ],
       'conditions': [
         ['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="solaris"', {
@@ -990,7 +939,8 @@
       'dependencies': [
         '../skia/skia.gyp:skia',
         '../third_party/libjingle/libjingle.gyp:libjingle',
-        'sync_proto',
+        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
+        '../third_party/protobuf2/protobuf.gyp:protobuf_lite#target',
       ],
       'conditions': [
         ['OS=="win"', {
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 1a7486a..dc046fe 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -17,7 +17,7 @@
         'domui_shared_resources',
         'net_internals_resources',
         'platform_locale_settings',
-        'sync_proto',
+        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'syncapi',
         'theme_resources',
         '../app/app.gyp:app_resources',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 0a9af8d..68bbca1 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -503,13 +503,13 @@
       'msvs_guid': 'ECFC2BEC-9FC0-4AD9-9649-5F26793F65FC',
       'dependencies': [
         'browser',
+        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'chrome',
         'chrome_resources',
         'chrome_strings',
         'common',
         'debugger',
         'renderer',
-        'sync_proto',
         'test_support_unit',
         'utility',
         '../app/app.gyp:app_base',
@@ -1715,6 +1715,7 @@
         '_USE_32BIT_TIME_T',
       ],
       'dependencies': [
+        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'common',
         'debugger',
         '../skia/skia.gyp:skia',
@@ -1722,7 +1723,6 @@
         '../testing/gtest.gyp:gtest',
         '../third_party/libjingle/libjingle.gyp:libjingle',
         'syncapi',
-        'sync_proto',
         'test_support_unit',
       ],
       'conditions': [
@@ -1810,15 +1810,14 @@
       'type': 'executable',
       'dependencies': [
         'browser',
+        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'chrome',
         'chrome_resources',
         'common',
         'debugger',
         'renderer',
-        'chrome_resources',
         'chrome_strings',
-        'sync_proto',
-        'test_support_unit',
+        'test_support_common',
         '../net/net.gyp:net_test_support',
         '../printing/printing.gyp:printing',
         '../skia/skia.gyp:skia',
@@ -1840,6 +1839,9 @@
         'app/chrome_dll_resource.h',
         'test/in_process_browser_test.cc',
         'test/in_process_browser_test.h',
+        'test/test_launcher/out_of_proc_test_runner.cc',
+        'test/test_launcher/test_runner.cc',
+        'test/test_launcher/test_runner.h',
         'test/live_sync/bookmark_model_verifier.cc',
         'test/live_sync/bookmark_model_verifier.h',
         'test/live_sync/live_sync_test.cc',
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index 451dd2e..941150e 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -93,12 +93,14 @@ void InProcessBrowserTest::SetUp() {
       "The user data directory name passed into this test was too "
       "short to delete safely.  Please check the user-data-dir "
       "argument and try again.";
-  if (ShouldDeleteProfile())
-    ASSERT_TRUE(file_util::DieFileDie(user_data_dir, true));
+  ASSERT_TRUE(file_util::DieFileDie(user_data_dir, true));
 
   // The unit test suite creates a testingbrowser, but we want the real thing.
   // Delete the current one. We'll install the testing one in TearDown.
   delete g_browser_process;
+  g_browser_process = NULL;
+
+  SetUpUserDataDirectory();
 
   // Don't delete the resources when BrowserMain returns. Many ui classes
   // cache SkBitmaps in a static field so that if we delete the resource
@@ -140,7 +142,7 @@ void InProcessBrowserTest::SetUp() {
                                       ASCIIToWide(kBrowserTestType));
 
   // Single-process mode is not set in BrowserMain so it needs to be processed
-  // explicitlty.
+  // explicitly.
   original_single_process_ = RenderProcessHost::run_renderer_in_process();
   if (command_line->HasSwitch(switches::kSingleProcess))
     RenderProcessHost::set_run_renderer_in_process(true);
@@ -279,7 +281,9 @@ void InProcessBrowserTest::RunTestOnMainThreadLoopDeprecated() {
       NewRunnableMethod(this, &InProcessBrowserTest::TimedOut),
       initial_timeout_);
 
-  RunTestOnMainThread();
+  // If an ASSERT_ failed during SetUp, skip the InProcessBrowserTest test body.
+  if (!HasFatalFailure())
+    RunTestOnMainThread();
   CleanUpOnMainThread();
 
   // Close all browser windows.  This might not happen immediately, since some
diff --git a/chrome/test/in_process_browser_test.h b/chrome/test/in_process_browser_test.h
index 2b63b7f..63cad3b 100644
--- a/chrome/test/in_process_browser_test.h
+++ b/chrome/test/in_process_browser_test.h
@@ -56,10 +56,6 @@ class InProcessBrowserTest : public testing::Test {
   // Restores state configured in SetUp.
   virtual void TearDown();
 
-  // This method is used to decide if user data dir
-  // needs to be deleted or not.
-  virtual bool ShouldDeleteProfile() { return true; }
-
  protected:
   // Returns the browser created by CreateBrowser.
   Browser* browser() const { return browser_; }
@@ -67,6 +63,12 @@ class InProcessBrowserTest : public testing::Test {
   // Override this rather than TestBody.
   virtual void RunTestOnMainThread() = 0;
 
+  // Helper to initialize the user data directory.  Called by SetUp() after
+  // erasing the user data directory, but before any browser is launched.
+  // If a test wishes to set up some initial non-empty state in the user
+  // data directory before the browser starts up, it can do so here.
+  virtual void SetUpUserDataDirectory() {};
+
   // We need these special methods because InProcessBrowserTest::SetUp is the
   // bottom of the stack that winds up calling your test method, so it is not
   // always an option to do what you want by overriding it and calling the
diff --git a/chrome/test/live_sync/live_sync_test.h b/chrome/test/live_sync/live_sync_test.h
index 9fd3373..cb11795 100644
--- a/chrome/test/live_sync/live_sync_test.h
+++ b/chrome/test/live_sync/live_sync_test.h
@@ -12,6 +12,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "googleurl/src/gurl.h"
+#include "net/socket/ssl_test_util.h"
 
 class BookmarkModel;
 class BookmarkNode;
@@ -45,10 +46,30 @@ class LiveSyncTest : public InProcessBrowserTest {
     ASSERT_FALSE(password_.empty()) << "Can't run live server test "
         << "without specifying --" << switches::kSyncPasswordForTest;
 
+    // Unless a sync server was explicitly provided, run a test one locally.
+    // TODO(ncarter): It might be better to allow the user to specify a choice
+    // of sync server "providers" -- a script that could locate (or allocate)
+    // a sync server instance, possibly on some remote host.  The provider
+    // would be invoked before each test.
+    if (!cl->HasSwitch(switches::kSyncServiceURL))
+      SetUpLocalTestServer();
+
     // Yield control back to the InProcessBrowserTest framework.
     InProcessBrowserTest::SetUp();
   }
 
+  virtual void SetUpLocalTestServer() {
+    bool success = server_.Start(net::TestServerLauncher::ProtoHTTP,
+        server_.kHostName, server_.kOKHTTPSPort,
+        FilePath(), FilePath(), std::wstring());
+    ASSERT_TRUE(success);
+
+    CommandLine* cl = CommandLine::ForCurrentProcess();
+    cl->AppendSwitchWithValue(switches::kSyncServiceURL,
+        StringPrintf("http://%s:%d/chromiumsync", server_.kHostName,
+            server_.kOKHTTPSPort));
+  }
+
   // Append command line flag to enable sync.
   virtual void SetUpCommandLine(CommandLine* command_line) {
   }
@@ -78,6 +99,8 @@ class LiveSyncTest : public InProcessBrowserTest {
   // to override the default resolver while the test is active.
   scoped_ptr<net::ScopedDefaultHostResolverProc> mock_host_resolver_override_;
 
+  net::TestServerLauncher server_;
+
   DISALLOW_COPY_AND_ASSIGN(LiveSyncTest);
 };
 
diff --git a/chrome/test/live_sync/two_client_live_bookmarks_sync_test.cc b/chrome/test/live_sync/two_client_live_bookmarks_sync_test.cc
index 47300bf..62f5b37 100644
--- a/chrome/test/live_sync/two_client_live_bookmarks_sync_test.cc
+++ b/chrome/test/live_sync/two_client_live_bookmarks_sync_test.cc
@@ -50,11 +50,6 @@ class TwoClientLiveBookmarksSyncTest : public LiveSyncTest {
     }
   }
 
-  // Overwrites ShouldDeleteProfile, so profile doesn't get deleted.
-  virtual bool ShouldDeleteProfile() {
-    return false;
-  }
-
   // Overload this method in inherited class and return false to avoid
   // race condition (two clients trying to sync/commit at the same time).
   // Race condition may lead to duplicate bookmarks if there is existing
@@ -94,7 +89,8 @@ class TwoClientLiveBookmarksSyncTest : public LiveSyncTest {
         FILE_PATH_LITERAL("live_sync_data"));
     FilePath source_file = sync_data_source.Append(
         bookmarks_file_name);
-    ASSERT_TRUE(file_util::PathExists(source_file));
+    ASSERT_TRUE(file_util::PathExists(source_file))
+        << "Could not locate test data file: " << source_file.value();
     // Now copy pre-generated bookmark file to default profile.
     ASSERT_TRUE(file_util::CopyFile(source_file,
         dest_user_data_dir_default.Append(FILE_PATH_LITERAL("bookmarks"))));
@@ -151,9 +147,10 @@ class LiveSyncTestPrePopulatedHistory1K
   LiveSyncTestPrePopulatedHistory1K() {}
   virtual ~LiveSyncTestPrePopulatedHistory1K() {}
 
-  // This is used to pre-populate history data (1K URL Visit)to Client1
-  // and Verifier Client.
-  void PrePopulateHistory1K() {
+  // This is used to pre-populate history data (1K URL Visit) to Client1
+  // and Verifier Client. Invoked by InProcBrowserTest::SetUp before
+  // the browser is started.
+  virtual void SetUpUserDataDirectory() {
     // Let's copy history files to default profile.
     FilePath dest_user_data_dir;
     PathService::Get(chrome::DIR_USER_DATA, &dest_user_data_dir);
@@ -182,11 +179,6 @@ class LiveSyncTestPrePopulatedHistory1K
     }
   }
 
-  virtual void SetUp() {
-    PrePopulateHistory1K();
-    LiveSyncTest::SetUp();
-  }
-
  private:
   DISALLOW_COPY_AND_ASSIGN(LiveSyncTestPrePopulatedHistory1K);
 };
@@ -197,10 +189,10 @@ class LiveSyncTestBasicHierarchy50BM
   LiveSyncTestBasicHierarchy50BM() {}
   virtual ~LiveSyncTestBasicHierarchy50BM() {}
 
-  virtual void SetUp() {
+  // Invoked by InProcBrowserTest::SetUp before the browser is started.
+  virtual void SetUpUserDataDirectory() {
     FilePath file_name(FILE_PATH_LITERAL("bookmarks_50BM5F3L"));
     PrePopulateBookmarksHierarchy(file_name);
-    LiveSyncTest::SetUp();
   }
 
  private:
@@ -233,10 +225,11 @@ class LiveSyncTestComplexHierarchy800BM
  public:
   LiveSyncTestComplexHierarchy800BM() {}
   virtual ~LiveSyncTestComplexHierarchy800BM() {}
-  virtual void SetUp() {
+
+  // Invoked by InProcBrowserTest::SetUp before the browser is started.
+  virtual void SetUpUserDataDirectory() {
     FilePath file_name(FILE_PATH_LITERAL("bookmarks_800BM32F8L"));
-    TwoClientLiveBookmarksSyncTest::PrePopulateBookmarksHierarchy(file_name);
-    LiveSyncTest::SetUp();
+    PrePopulateBookmarksHierarchy(file_name);
   }
 
  private:
@@ -248,10 +241,11 @@ class LiveSyncTestHugeHierarchy5500BM
  public:
   LiveSyncTestHugeHierarchy5500BM() {}
   virtual ~LiveSyncTestHugeHierarchy5500BM() {}
-  virtual void SetUp() {
+
+  // Invoked by InProcBrowserTest::SetUp before the browser is started.
+  virtual void SetUpUserDataDirectory() {
     FilePath file_name(FILE_PATH_LITERAL("bookmarks_5500BM125F25L"));
-    TwoClientLiveBookmarksSyncTest::PrePopulateBookmarksHierarchy(file_name);
-    LiveSyncTest::SetUp();
+    PrePopulateBookmarksHierarchy(file_name);
   }
   virtual bool ShouldSetupSyncWithRace() {
     return false;
@@ -267,11 +261,11 @@ class LiveSyncTestDefaultIEFavorites
   LiveSyncTestDefaultIEFavorites() {}
   virtual ~LiveSyncTestDefaultIEFavorites() {}
 
-  virtual void SetUp() {
+  // Invoked by InProcBrowserTest::SetUp before the browser is started.
+  virtual void SetUpUserDataDirectory() {
     const FilePath file_name(
         FILE_PATH_LITERAL("bookmarks_default_IE_favorites"));
     TwoClientLiveBookmarksSyncTest::PrePopulateBookmarksHierarchy(file_name);
-    LiveSyncTest::SetUp();
   }
 
  private:
diff --git a/chrome/test/memory_test/memory_test.cc b/chrome/test/memory_test/memory_test.cc
index ef1eebc..079e0a4 100644
--- a/chrome/test/memory_test/memory_test.cc
+++ b/chrome/test/memory_test/memory_test.cc
@@ -143,7 +143,7 @@ class MemoryTest : public UITest {
     int expected_tab_count = 1;
     for (unsigned counter = 0; counter < urls_length; ++counter) {
       std::string url = urls[counter];
-      
+
       SCOPED_TRACE(url);
 
       if (url == "<PAUSE>") {  // Special command to delay on this page