Revert 44708 - Python implementation of sync server, for testing.

Implement the server side of chromium sync inside of testserver.py.  The implementation supports at most one account (and ignores authentication credentials), but is otherwise reasonably full featured.

Make the sync_integration_tests run by default against the test server.  An externallyprovided syncurl will give the old behavior.

Protocol buffers stuff: The test sync server requires Python generated code for .proto files.  I've put generated code, as well as the python protocol buffers runtime library, in the output directory + "/python" (e.g, on windows, src/chrome/Debug/python/google/protobuf).  This might be a bad place, or a bad name  open to suggestions.  The main thing is that there will be runtime dependencies on it, so it didn't seem right to put it in obj/.

Flakiness fix: In the InProcessBrowserTest framework, improve the mechanism for tests that want to manually set up a user data directory.  The new way ensures that the user data directory is always wiped; tests can't accidentally forget to do this anymore.

Flakiness fix: Make testserver try to /kill any old instance that might be hogging the port.  Very useful if a test failure leaves a server running.  Tested this against all combos of protocols, and it seems to work.

Flakiness fix: Port sync_integration_tests to the outofprocess test runner.

Flakiness fix: For IN_PROC_BROWSER_TESTS, don't run the test body if the setup triggered a fatal (ASSERT_) failure.

BUG=20905,40980

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

TBR=nick@chromium.org
Review URL: http://codereview.chromium.org/1633015

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

10 files changed, 94 insertions, 142 deletions

diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 1278e43..4781619 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -497,7 +497,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
deleted file mode 100755
index 1bc15ad..0000000
--- a/chrome/browser/sync/protocol/sync_proto.gyp
+++ /dev/null
@@ -1,73 +0,0 @@
-# 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': [
-            '<(SHARED_INTERMEDIATE_DIR)/protoc_out/chrome/browser/sync/protocol/<(RULE_INPUT_ROOT).pb.h',
-            '<(SHARED_INTERMEDIATE_DIR)/protoc_out/proto/chrome/browser/sync/protocol/<(RULE_INPUT_ROOT).pb.cc',
-            '<(PRODUCT_DIR)/pyproto/sync_pb/<(RULE_INPUT_ROOT)_pb2.py',
-          ],
-          '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 426f86d..c9b2698 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -689,10 +689,61 @@
         '../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',
       ],
     },
     {
@@ -782,7 +833,7 @@
       'dependencies': [
         '../third_party/expat/expat.gyp:expat',
         '../third_party/libjingle/libjingle.gyp:libjingle',
-        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
+        'sync_proto',
       ],
       'conditions': [
         ['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="solaris"', {
@@ -933,8 +984,7 @@
       'dependencies': [
         '../skia/skia.gyp:skia',
         '../third_party/libjingle/libjingle.gyp:libjingle',
-        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
-        '../third_party/protobuf2/protobuf.gyp:protobuf_lite#target',
+        'sync_proto',
       ],
       'conditions': [
         ['OS=="win"', {
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 9f78335..78bda8a 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -16,7 +16,7 @@
         'component_extensions',
         'net_internals_resources',
         'platform_locale_settings',
-        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
+        'sync_proto',
         'syncapi',
         'theme_resources',
         '../app/app.gyp:app_resources',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 647a8d0..94070dfe 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -496,13 +496,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',
@@ -1700,7 +1700,6 @@
         '_USE_32BIT_TIME_T',
       ],
       'dependencies': [
-        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'common',
         'debugger',
         '../skia/skia.gyp:skia',
@@ -1708,6 +1707,7 @@
         '../testing/gtest.gyp:gtest',
         '../third_party/libjingle/libjingle.gyp:libjingle',
         'syncapi',
+        'sync_proto',
         'test_support_unit',
       ],
       'conditions': [
@@ -1758,14 +1758,15 @@
       'type': 'executable',
       'dependencies': [
         'browser',
-        'browser/sync/protocol/sync_proto.gyp:sync_proto_cpp',
         'chrome',
         'chrome_resources',
         'common',
         'debugger',
         'renderer',
+        'chrome_resources',
         'chrome_strings',
-        'test_support_common',
+        'sync_proto',
+        'test_support_unit',
         '../net/net.gyp:net_test_support',
         '../printing/printing.gyp:printing',
         '../skia/skia.gyp:skia',
@@ -1787,9 +1788,6 @@
         '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 ee5923e..7463476 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -93,14 +93,12 @@ 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.";
-  ASSERT_TRUE(file_util::DieFileDie(user_data_dir, true));
+  if (ShouldDeleteProfile())
+    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
@@ -142,7 +140,7 @@ void InProcessBrowserTest::SetUp() {
                                       ASCIIToWide(kBrowserTestType));
 
   // Single-process mode is not set in BrowserMain so it needs to be processed
-  // explicitly.
+  // explicitlty.
   original_single_process_ = RenderProcessHost::run_renderer_in_process();
   if (command_line->HasSwitch(switches::kSingleProcess))
     RenderProcessHost::set_run_renderer_in_process(true);
@@ -280,9 +278,7 @@ void InProcessBrowserTest::RunTestOnMainThreadLoopDeprecated() {
       NewRunnableMethod(this, &InProcessBrowserTest::TimedOut),
       initial_timeout_);
 
-  // If an ASSERT_ failed during SetUp, skip the InProcessBrowserTest test body.
-  if (!HasFatalFailure())
-    RunTestOnMainThread();
+  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 ac9844b..0c26123 100644
--- a/chrome/test/in_process_browser_test.h
+++ b/chrome/test/in_process_browser_test.h
@@ -56,6 +56,10 @@ 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_; }
@@ -63,12 +67,6 @@ 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 ec8a5c6..43975f6 100644
--- a/chrome/test/live_sync/live_sync_test.h
+++ b/chrome/test/live_sync/live_sync_test.h
@@ -12,7 +12,6 @@
 #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;
@@ -46,30 +45,10 @@ 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) {
   }
@@ -99,8 +78,6 @@ 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 9e7ff03..bfae3c7 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,6 +50,11 @@ 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
@@ -89,8 +94,7 @@ 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))
-        << "Could not locate test data file: " << source_file.value();
+    ASSERT_TRUE(file_util::PathExists(source_file));
     // 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"))));
@@ -147,10 +151,9 @@ class LiveSyncTestPrePopulatedHistory1K
   LiveSyncTestPrePopulatedHistory1K() {}
   virtual ~LiveSyncTestPrePopulatedHistory1K() {}
 
-  // 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() {
+  // This is used to pre-populate history data (1K URL Visit)to Client1
+  // and Verifier Client.
+  void PrePopulateHistory1K() {
     // Let's copy history files to default profile.
     FilePath dest_user_data_dir;
     PathService::Get(chrome::DIR_USER_DATA, &dest_user_data_dir);
@@ -179,6 +182,11 @@ class LiveSyncTestPrePopulatedHistory1K
     }
   }
 
+  virtual void SetUp() {
+    PrePopulateHistory1K();
+    LiveSyncTest::SetUp();
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(LiveSyncTestPrePopulatedHistory1K);
 };
@@ -189,10 +197,10 @@ class LiveSyncTestBasicHierarchy50BM
   LiveSyncTestBasicHierarchy50BM() {}
   virtual ~LiveSyncTestBasicHierarchy50BM() {}
 
-  // Invoked by InProcBrowserTest::SetUp before the browser is started.
-  virtual void SetUpUserDataDirectory() {
+  virtual void SetUp() {
     FilePath file_name(FILE_PATH_LITERAL("bookmarks_50BM5F3L"));
     PrePopulateBookmarksHierarchy(file_name);
+    LiveSyncTest::SetUp();
   }
 
  private:
@@ -225,11 +233,10 @@ class LiveSyncTestComplexHierarchy800BM
  public:
   LiveSyncTestComplexHierarchy800BM() {}
   virtual ~LiveSyncTestComplexHierarchy800BM() {}
-
-  // Invoked by InProcBrowserTest::SetUp before the browser is started.
-  virtual void SetUpUserDataDirectory() {
+  virtual void SetUp() {
     FilePath file_name(FILE_PATH_LITERAL("bookmarks_800BM32F8L"));
-    PrePopulateBookmarksHierarchy(file_name);
+    TwoClientLiveBookmarksSyncTest::PrePopulateBookmarksHierarchy(file_name);
+    LiveSyncTest::SetUp();
   }
 
  private:
@@ -241,11 +248,10 @@ class LiveSyncTestHugeHierarchy5500BM
  public:
   LiveSyncTestHugeHierarchy5500BM() {}
   virtual ~LiveSyncTestHugeHierarchy5500BM() {}
-
-  // Invoked by InProcBrowserTest::SetUp before the browser is started.
-  virtual void SetUpUserDataDirectory() {
+  virtual void SetUp() {
     FilePath file_name(FILE_PATH_LITERAL("bookmarks_5500BM125F25L"));
-    PrePopulateBookmarksHierarchy(file_name);
+    TwoClientLiveBookmarksSyncTest::PrePopulateBookmarksHierarchy(file_name);
+    LiveSyncTest::SetUp();
   }
   virtual bool ShouldSetupSyncWithRace() {
     return false;
@@ -261,11 +267,11 @@ class LiveSyncTestDefaultIEFavorites
   LiveSyncTestDefaultIEFavorites() {}
   virtual ~LiveSyncTestDefaultIEFavorites() {}
 
-  // Invoked by InProcBrowserTest::SetUp before the browser is started.
-  virtual void SetUpUserDataDirectory() {
+  virtual void SetUp() {
     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 079e0a4..ef1eebc 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