Upstream Android proxy config service.

net/proxy/proxy_config_service_android_unittest.cc

+// 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.
+
+#include <map>
+#include <string>
+
+#include "base/bind.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "net/proxy/proxy_config.h"
+#include "net/proxy/proxy_config_service_android.h"
+#include "net/proxy/proxy_info.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+class TestObserver : public ProxyConfigService::Observer {
+ public:
+  TestObserver() : availability_(ProxyConfigService::CONFIG_UNSET) {}
+
+  // ProxyConfigService::Observer:
+  virtual void OnProxyConfigChanged(
+      const ProxyConfig& config,
+      ProxyConfigService::ConfigAvailability availability) OVERRIDE {
+    config_ = config;
+    availability_ = availability;
+  }
+
+  // Called from main thread.

[Ryan Sleevi, 2012/05/29 17:34:43]

nit: still applicable? No threads involved now

[Philippe, 2012/06/01 13:06:37]

Done.

+  ProxyConfigService::ConfigAvailability availability() const {
+    return availability_;
+  }
+
+  const ProxyConfig& config() const {
+    return config_;
+  }
+
+ private:
+  ProxyConfig config_;
+  ProxyConfigService::ConfigAvailability availability_;
+};
+
+std::string ToString(const ProxyInfo& proxy_info) {

[Ryan Sleevi, 2012/05/29 17:34:43]

The naming of this method is not ideal - but more importantly, it's also not
clear why it's needed. It only seems to be used on line 111, in which case, just
call proxy_info.ToPacString()

If it is part of some auto-testing piece, then ProxyInfoToPacString would be
better, if only for verbosity sake.

[Philippe, 2012/06/01 13:06:37]

Not needed (anymore) indeed.

+  return proxy_info.ToPacString();
+}
+
+}  // namespace
+
+class ProxyConfigServiceAndroidTest : public testing::Test {
+ protected:
+  ProxyConfigServiceAndroidTest() : service_(NULL) {}
+
+  // testing::Test:
+  virtual void SetUp() OVERRIDE {

[Ryan Sleevi, 2012/05/29 17:34:43]

Note: Every test runs as a new instance, so you do not need to put this in
SetUp(). Instead, you can move much of this to being in the constructor, with
the variables declared on the stack, rather than on the heap.

ProxyConfigServiceAndroidTest()
  : service_(message_loop_.message_loop_proxy(),
             message_loop_.message_loop_proxy(),
             base::Bind(...)) {
}

virtual void SetUp() OVERRIDE {
  message_loop.RunAllPending();
  service_.AddObserver(&observer);
}

virtual void TearDown() OVERRIDE {
  service_.RemoveObserver(&observer);
}

[Philippe, 2012/06/01 13:06:37]

I didn't know that a new instance was created for each unit test. Thanks for the
information.

+    observer_.reset(new TestObserver());
+    message_loop_ = MessageLoop::current();

[Ryan Sleevi, 2012/05/29 17:34:43]

BUG: There is no guarantee that there will be a current MessageLoop at the time
this test runs. You need to explicitly create a MessageLoop (heap or stack)
before assuming MessageLoop::current().

If there is a MessageLoop::current(), one or more tests may be leaking. The
general proof of this is to run with a --gtest_filter for just this test; it
should presumably crash.

Or is this part of the JNI test runner infrastructure, where net_unittests is
built as a shared library? If so, please clearly comment about why
MessageLoop::current() is safe in this test.

[Philippe, 2012/06/01 13:06:37]

I created a new message loop. Thanks for the details.

+    service_.reset(new ProxyConfigServiceAndroid(
+        message_loop_->message_loop_proxy(),
+        message_loop_->message_loop_proxy(),
+        base::Bind(&ProxyConfigServiceAndroidTest::GetProperty,
+                   base::Unretained(this))));
+    message_loop_->RunAllPending();
+    service_->AddObserver(observer_.get());
+  }
+
+  virtual void TearDown() OVERRIDE {
+    ClearConfiguration();
+    service_.reset();

[Ryan Sleevi, 2012/05/29 17:34:43]

BUG: You don't RemoveObserver (line 67)

[Philippe, 2012/06/01 13:06:37]

Done.

+    message_loop_ = NULL;
+  }
+
+  void ClearConfiguration() {
+    configuration_.clear();
+  }
+
+  void AddProperty(const std::string& key, const std::string& value) {
+    configuration_[key] = value;
+  }
+
+  std::string GetProperty(const std::string& key) {
+    StringMap::const_iterator it = configuration_.find(key);
+    if (it == configuration_.end())
+      return std::string();
+    return it->second;
+  }
+
+  void ProxySettingsChanged() {
+    service_->ProxySettingsChanged(NULL, NULL);
+    message_loop_->RunAllPending();
+  }
+
+  void TestMapping(const std::string& url, const std::string& expected) {
+    ProxyConfigService::ConfigAvailability availability;
+    ProxyConfig proxy_config;
+    availability = service_->GetLatestProxyConfig(&proxy_config);
+    TestMappingForProxyConfig(url, expected, availability, &proxy_config);
+  }
+
+  static void TestMappingForProxyConfig(
+      const std::string& url,
+      const std::string& expected,
+      ProxyConfigService::ConfigAvailability availability,
+      ProxyConfig* proxy_config) {

[Ryan Sleevi, 2012/05/29 17:34:43]

nit: You only seem to call this method on line 100. Should this just be rolled
into the above TestMapping?

[Philippe, 2012/06/01 13:06:37]

Indeed. I had a unit test using it before.

+    EXPECT_EQ(ProxyConfigService::CONFIG_VALID, availability);
+    ProxyInfo proxy_info;
+    proxy_config->proxy_rules().Apply(GURL(url), &proxy_info);
+    EXPECT_EQ(expected, ToString(proxy_info));
+  }
+
+  scoped_ptr<TestObserver> observer_;
+  scoped_ptr<ProxyConfigServiceAndroid> service_;
+
+ private:
+  typedef std::map<std::string, std::string> StringMap;

[Ryan Sleevi, 2012/05/29 17:34:43]

Is this typedef still necessary? Seems to only be used on line 85

[Philippe, 2012/06/01 13:06:37]

It wasn't indeed. I had to keep it though since I'm now using it more often.

+  StringMap configuration_;
+
+  MessageLoop* message_loop_;
+};
+
+class ProxyConfigServiceAndroidWithInitialConfigTest
+    : public ProxyConfigServiceAndroidTest {
+ protected:
+  // ProxyConfigServiceAndroidTest:
+  virtual void SetUp() OVERRIDE {
+    AddProperty("http.proxyHost", "httpproxy.com");
+    AddProperty("http.proxyPort", "8080");
+    ProxyConfigServiceAndroidTest::SetUp();

[Ryan Sleevi, 2012/05/29 17:34:43]

nit: You should generally always call the parent method first. 

[Philippe, 2012/06/01 13:06:37]

This is a little subtle. I set the configuration before so that when the service
is constructed, in ProxyConfigServiceAndroidTest::SetUp(), and more importantly
the initial config fetched, we use a non-empty configuration map.
I had to make some small changes here anyway since we don't create the service
in SetUp() anymore.

+  }
+};
+
+TEST_F(ProxyConfigServiceAndroidTest, TestChangePropertiesNotification) {
+  // Set up a non-empty configuration
+  AddProperty("http.proxyHost", "localhost");
+  ProxySettingsChanged();
+  EXPECT_EQ(ProxyConfigService::CONFIG_VALID, observer_->availability());
+  EXPECT_FALSE(observer_->config().proxy_rules().empty());
+
+  // Set up an empty configuration
+  ClearConfiguration();
+  ProxySettingsChanged();
+  EXPECT_EQ(ProxyConfigService::CONFIG_VALID, observer_->availability());
+  EXPECT_TRUE(observer_->config().proxy_rules().empty());
+}
+
+TEST_F(ProxyConfigServiceAndroidWithInitialConfigTest, TestInitialConfig) {
+  // Make sure that the initial config is set.
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "PROXY httpproxy.com:8080");
+
+  // Override the initial configuration.
+  ClearConfiguration();
+  AddProperty("http.proxyHost", "httpproxy.com");
+  ProxySettingsChanged();
+  TestMapping("http://example.com/", "PROXY httpproxy.com:80");
+}
+
+// !! The following test cases are automatically generated from
+// !! net/android/tools/proxy_test_cases.py.
+// !! Please edit that file instead of editing the test cases below and
+// !! update also the corresponding Java unit tests in
+// !! AndroidProxySelectorTest.java
+
+TEST_F(ProxyConfigServiceAndroidTest, NoProxy) {
+  // Test direct mapping when no proxy defined.
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "DIRECT");
+  TestMapping("https://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpProxyHostAndPort) {
+  // Test http.proxyHost and http.proxyPort works.
+  AddProperty("http.proxyHost", "httpproxy.com");
+  AddProperty("http.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "PROXY httpproxy.com:8080");
+  TestMapping("https://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpProxyHostOnly) {
+  // We should get the default port (80) for proxied hosts.
+  AddProperty("http.proxyHost", "httpproxy.com");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "PROXY httpproxy.com:80");
+  TestMapping("https://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpProxyPortOnly) {
+  // http.proxyPort only should not result in any hosts being proxied.
+  AddProperty("http.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "DIRECT");
+  TestMapping("https://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpNonProxyHosts1) {
+  // Test that HTTP non proxy hosts are mapped correctly
+  AddProperty("http.nonProxyHosts", "slashdot.org");
+  AddProperty("http.proxyHost", "httpproxy.com");
+  AddProperty("http.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("http://example.com/", "PROXY httpproxy.com:8080");
+  TestMapping("http://slashdot.org/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpNonProxyHosts2) {
+  // Test that | pattern works.
+  AddProperty("http.nonProxyHosts", "slashdot.org|freecode.net");
+  AddProperty("http.proxyHost", "httpproxy.com");
+  AddProperty("http.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("http://example.com/", "PROXY httpproxy.com:8080");
+  TestMapping("http://freecode.net/", "DIRECT");
+  TestMapping("http://slashdot.org/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpNonProxyHosts3) {
+  // Test that * pattern works.
+  AddProperty("http.nonProxyHosts", "*example.com");
+  AddProperty("http.proxyHost", "httpproxy.com");
+  AddProperty("http.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("http://example.com/", "DIRECT");
+  TestMapping("http://slashdot.org/", "PROXY httpproxy.com:8080");
+  TestMapping("http://www.example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, FtpNonProxyHosts) {
+  // Test that FTP non proxy hosts are mapped correctly
+  AddProperty("ftp.nonProxyHosts", "slashdot.org");
+  AddProperty("ftp.proxyHost", "httpproxy.com");
+  AddProperty("ftp.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "PROXY httpproxy.com:8080");
+  TestMapping("http://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, FtpProxyHostAndPort) {
+  // Test ftp.proxyHost and ftp.proxyPort works.
+  AddProperty("ftp.proxyHost", "httpproxy.com");
+  AddProperty("ftp.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "PROXY httpproxy.com:8080");
+  TestMapping("http://example.com/", "DIRECT");
+  TestMapping("https://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, FtpProxyHostOnly) {
+  // Test ftp.proxyHost and default port.
+  AddProperty("ftp.proxyHost", "httpproxy.com");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "PROXY httpproxy.com:80");
+  TestMapping("http://example.com/", "DIRECT");
+  TestMapping("https://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpsProxyHostAndPort) {
+  // Test https.proxyHost and https.proxyPort works.
+  AddProperty("https.proxyHost", "httpproxy.com");
+  AddProperty("https.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "DIRECT");
+  TestMapping("https://example.com/", "HTTPS httpproxy.com:8080");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpsProxyHostOnly) {
+  // Test https.proxyHost and default port.
+  AddProperty("https.proxyHost", "httpproxy.com");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "DIRECT");
+  TestMapping("https://example.com/", "HTTPS httpproxy.com:443");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpProxyHostIPv6) {
+  // Test IPv6 https.proxyHost and default port.
+  AddProperty("http.proxyHost", "a:b:c::d:1");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "PROXY [a:b:c::d:1]:80");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpProxyHostAndPortIPv6) {
+  // Test IPv6 http.proxyHost and http.proxyPort works.
+  AddProperty("http.proxyHost", "a:b:c::d:1");
+  AddProperty("http.proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "PROXY [a:b:c::d:1]:8080");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpProxyHostAndInvalidPort) {
+  // Test invalid http.proxyPort does not crash.
+  AddProperty("http.proxyHost", "a:b:c::d:1");
+  AddProperty("http.proxyPort", "65536");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "DIRECT");
+  TestMapping("http://example.com/", "DIRECT");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, DefaultProxyExplictPort) {
+  // Default http proxy is used if a scheme-specific one is not found.
+  AddProperty("ftp.proxyHost", "httpproxy.com");
+  AddProperty("ftp.proxyPort", "8080");
+  AddProperty("proxyHost", "defaultproxy.com");
+  AddProperty("proxyPort", "8080");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com/", "PROXY httpproxy.com:8080");
+  TestMapping("http://example.com/", "PROXY defaultproxy.com:8080");
+  TestMapping("https://example.com/", "HTTPS defaultproxy.com:8080");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, DefaultProxyDefaultPort) {
+  // Check that the default proxy port is as expected.
+  AddProperty("proxyHost", "defaultproxy.com");
+  ProxySettingsChanged();
+  TestMapping("http://example.com/", "PROXY defaultproxy.com:80");
+  TestMapping("https://example.com/", "HTTPS defaultproxy.com:443");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, FallbackToSocks) {
+  // SOCKS proxy is used if scheme-specific one is not found.
+  AddProperty("http.proxyHost", "defaultproxy.com");
+  AddProperty("socksProxyHost", "socksproxy.com");
+  ProxySettingsChanged();
+  TestMapping("ftp://example.com", "SOCKS5 socksproxy.com:1080");
+  TestMapping("http://example.com/", "PROXY defaultproxy.com:80");
+  TestMapping("https://example.com/", "SOCKS5 socksproxy.com:1080");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, SocksExplicitPort) {
+  // SOCKS proxy port is used if specified
+  AddProperty("socksProxyHost", "socksproxy.com");
+  AddProperty("socksProxyPort", "9000");
+  ProxySettingsChanged();
+  TestMapping("http://example.com/", "SOCKS5 socksproxy.com:9000");
+}
+
+TEST_F(ProxyConfigServiceAndroidTest, HttpProxySupercedesSocks) {
+  // SOCKS proxy is ignored if default HTTP proxy defined.
+  AddProperty("proxyHost", "defaultproxy.com");
+  AddProperty("socksProxyHost", "socksproxy.com");
+  AddProperty("socksProxyPort", "9000");
+  ProxySettingsChanged();
+  TestMapping("http://example.com/", "PROXY defaultproxy.com:80");
+}
+
+}  // namespace net