Eliminated the logic that accumulated multiple preconnect requests

chrome/browser/net/predictor_browsertest.cc

Index: chrome/browser/net/predictor_browsertest.cc
diff --git a/chrome/browser/net/predictor_browsertest.cc b/chrome/browser/net/predictor_browsertest.cc
index 307c61c25604fb0f7ceb9c9982de881fc264bf9c..eb35e46975b320e29a5dbba943dd67b8a5318f61 100644
--- a/chrome/browser/net/predictor_browsertest.cc
+++ b/chrome/browser/net/predictor_browsertest.cc
@@ -2,8 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/base64.h"
+#include "base/command_line.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/prefs/pref_service.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -11,7 +15,9 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/test_utils.h"
+#include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_log.h"
 #include "net/dns/host_resolver_proc.h"
 #include "net/dns/mock_host_resolver.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -21,6 +27,9 @@ using testing::HasSubstr;
 
 namespace {
 
+const char kEnablePreconnect[] = "--enable-blink-features=LinkPreconnect";
+const char kEnableExperimentalWebPlatformFeatures[] =
+    "--enable-experimental-web-platform-features";

[mmenke, 2015/02/17 16:53:33]

Should get these strings from the header (public/common/content_switches.h),
rather than duplicate them here.  That way, if they're ever changed or remove,
get a compile error instead of a test error.  Looks like you'll still need to
have "LinkPreconnect" explicitly in this file.

[Pat Meenan, 2015/02/17 17:38:08]

Done.

 const char kChromiumHostname[] = "chromium.org";
 
 // Records a history of all hostnames for which resolving has been requested,
@@ -91,6 +100,55 @@ class HostResolutionRequestRecorder : public net::HostResolverProc {
   DISALLOW_COPY_AND_ASSIGN(HostResolutionRequestRecorder);
 };
 
+// Watches the NetLog event stream for a connect event to the provided
+// host:port pair.
+class ConnectNetLogObserver : public net::NetLog::ThreadSafeObserver {
+ public:
+  explicit ConnectNetLogObserver(const std::string& host_port_pair)
+      : host_port_pair_(host_port_pair),
+        saw_connect_event_(false) {
+  }
+
+  ~ConnectNetLogObserver() override {
+  }
+
+  void Attach() {
+    g_browser_process->net_log()->AddThreadSafeObserver(
+        this, net::NetLog::LOG_ALL_BUT_BYTES);
+  }
+
+  void Detach() {
+    if (net_log())
+      net_log()->RemoveThreadSafeObserver(this);
+  }
+
+  void WaitForConnect() {
+    if (!saw_connect_event_)

[mmenke, 2015/02/17 16:53:33]

Per earlier comment, need to get rid of this.  RunLoop handles quit before start
correctly (Never starts the loop, I believe), and this isn't currently
threadsafe.

[Pat Meenan, 2015/02/17 17:38:07]

Done.

+      run_loop_.Run();
+  }
+
+ private:
+  void OnAddEntry(const net::NetLog::Entry& entry) override {
+    scoped_ptr<base::Value> param_value(entry.ParametersToValue());
+    base::DictionaryValue* param_dict = NULL;
+    std::string group_name;
+
+    if (entry.source().type == net::NetLog::SOURCE_CONNECT_JOB &&
+        param_value.get() != NULL &&
+        param_value->GetAsDictionary(&param_dict) &&
+        param_dict != NULL &&
+        param_dict->GetString("group_name", &group_name) &&
+        host_port_pair_ == group_name) {
+      saw_connect_event_ = true;
+      run_loop_.Quit();
+    }
+  }
+
+  base::RunLoop run_loop_;
+  const std::string host_port_pair_;
+  bool saw_connect_event_;
+};
+
 }  // namespace
 
 namespace chrome_browser_net {
@@ -111,6 +169,11 @@ class PredictorBrowserTest : public InProcessBrowserTest {
     InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
   }
 
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(kEnablePreconnect);
+    command_line->AppendSwitch(kEnableExperimentalWebPlatformFeatures);
+  }
+
   void TearDownInProcessBrowserTestFixture() override {
     InProcessBrowserTest::TearDownInProcessBrowserTestFixture();
     scoped_host_resolver_proc_.reset();
@@ -200,5 +263,28 @@ IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, DnsPrefetch) {
   WaitUntilHostHasBeenRequested(kChromiumHostname);
 }
 
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, Preconnect) {
+  ASSERT_TRUE(test_server()->Start());
+
+  // Create a HTML preconnect reference to the local server in the form
+  // <link rel="preconnect" href="http://test-server/">
+  // and navigate to it as a data URI.
+  GURL preconnect_url = test_server()->GetURL("");
+  std::string preconnect_content =
+      "<link rel=\"preconnect\" href=\"" + preconnect_url.spec() + "\">";
+  std::string encoded;
+  base::Base64Encode(preconnect_content, &encoded);
+  std::string data_uri = "data:text/html;base64," + encoded;
+
+  net::HostPortPair host_port_pair = net::HostPortPair::FromURL(preconnect_url);
+  ConnectNetLogObserver net_log_observer(host_port_pair.ToString());
+  net_log_observer.Attach();
+
+  ui_test_utils::NavigateToURL(browser(), GURL(data_uri));
+
+  net_log_observer.WaitForConnect();
+  net_log_observer.Detach();
+}
+
 }  // namespace chrome_browser_net