[sync] Add --port and --xmpp-port parameters to run_testserver.cc

net/test/base_test_server.h

 // Copyright (c) 2012 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.
 
 #ifndef NET_TEST_BASE_TEST_SERVER_H_
 #define NET_TEST_BASE_TEST_SERVER_H_
 #pragma once
 
 #include <string>
 #include <utility>
@@ skipping 164 matching lines @@
   // whether it's available (server started) or not.
   uint16 GetPort();
 
   // Sets |port| as the actual port used by Python based test server.
   void SetPort(uint16 port);
 
   // Set up internal status when the server is started.
   bool SetupWhenServerStarted() WARN_UNUSED_RESULT;
 
   // Clean up internal status when starting to stop server.
-  void CleanUpWhenStoppingServer();
+  virtual void CleanUpWhenStoppingServer();

[Paweł Hajdan Jr., 2012/05/23 06:53:02]

Please find a way to avoid making this virtual. Once we allow protected virtual
methods the contract between classes becomes more complex and fragile.

Would preventing the server from being started again after it's stopped break
anything in the codebase? If not, we could do that, and remove
CleanUpWhenStoppingServer almost completely, at least zeroing-out int member
variables.

[Raghu Simha, 2012/05/23 19:54:28]

Done. 

The sync server is only started once in every place it is used, so
CleanUpWhenStoppingServer is moot when it comes to sync. I've undone this
change.

Changes to how CleanUpWhenStoppingServer works (including its removal) is out of
the scope of this patch.

 
   // Set path of test resources.
   void SetResourcePath(const FilePath& document_root,
                        const FilePath& certificates_dir);
 
   // Parses the server data read from the test server.  Returns true
   // on success.
   bool ParseServerData(const std::string& server_data) WARN_UNUSED_RESULT;
 
   // Generates a DictionaryValue with the arguments for launching the external
   // Python test server.
-  bool GenerateArguments(base::DictionaryValue* arguments) const;
+  virtual bool GenerateArguments(base::DictionaryValue* arguments) const;

[Paweł Hajdan Jr., 2012/05/23 06:53:02]

Do not override this. See how LocalTestServer handles that.

[Raghu Simha, 2012/05/23 19:54:28]

Done.

There are two sets of arguments being strung together here -- the ones passed to
python.exe and the ones passed to testserver.py.
BaseTestServer::GenerateArguments takes care of the args passed to testserver.py
while LocalTestServer::AddCommandLineArguments takes care of the ones passed to
python.exe and then calls AppendArg for the stuff returned by
BaseTestServer::GenerateArguments. (AppendSwitchASCII won't work because the
args will then get passed to python.exe, not testserver.py)

The only way to avoid making BaseTestServer::GenerateArguments virtual is to
make LocalTestServer::AddCommandLineArguments public virtual and call it from
the override, LocalSyncTestServer::AddCommandLineArguments.

 
  private:
   void Init(const std::string& host);
 
   // Marks the root certificate of an HTTPS test server as trusted for
   // the duration of tests.
   bool LoadTestRootCert() const WARN_UNUSED_RESULT;
 
   // Document root of the test server.
   FilePath document_root_;
@@ skipping 20 matching lines @@
 
   scoped_ptr<ScopedPortException> allowed_port_;
 
   DISALLOW_COPY_AND_ASSIGN(BaseTestServer);
 };
 
 }  // namespace net
 
 #endif  // NET_TEST_BASE_TEST_SERVER_H_