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..7113c465328d7e47c7441b41287677519ec0d8c1 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";
 const char kChromiumHostname[] = "chromium.org";
 
 // Records a history of all hostnames for which resolving has been requested,
@@ -91,6 +100,50 @@ 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)

[mmenke, 2015/02/13 22:49:09]

nit:  const std::String&

[Pat Meenan, 2015/02/14 00:42:56]

Done.

+    : host_port_pair_(host_port_pair),

[mmenke, 2015/02/13 22:49:08]

nit:  +2 indent

[Pat Meenan, 2015/02/14 00:42:56]

Done.

+    saw_connect_event_(false) {

[mmenke, 2015/02/13 22:49:08]

nit:  +4 indent

[Pat Meenan, 2015/02/14 00:42:56]

Done.

+    g_browser_process->net_log()->AddThreadSafeObserver(
+        this, net::NetLog::LOG_ALL_BUT_BYTES);

[mmenke, 2015/02/13 22:49:08]

Adding during the constructor and removing during the destructor is just a bad
idea with the NetLog, due to threadsafety issues (The NetLog has some
protection, but if you start watching here, it could call into you on another
thread before your constructor completes, and down that path lies madness. 
Could call into the observer while in the middle of the destructor with the
removal there as well)

[Pat Meenan, 2015/02/14 00:42:56]

Done.

+  }
+
+  ~ConnectNetLogObserver() override {
+    if (net_log())
+      net_log()->RemoveThreadSafeObserver(this);
+  }
+
+  void WaitForConnect() {
+    if (!saw_connect_event_)

[mmenke, 2015/02/13 22:49:08]

This isn't needed.  Run loop can be quit before it's started and it works as
expected.  Moreover, OnAddEntry is called on the IO thread, so this isn't
threadsafe.

[Pat Meenan, 2015/02/14 00:42:56]

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;
+      if (run_loop_.running())
+        run_loop_.Quit();
+    }
+  }
+
+  base::RunLoop run_loop_;
+  const std::string host_port_pair_;
+  bool saw_connect_event_;
+};
+
 }  // namespace
 
 namespace chrome_browser_net {
@@ -111,6 +164,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 +258,26 @@ 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());
+
+  ui_test_utils::NavigateToURL(browser(), GURL(data_uri));
+
+  net_log_observer.WaitForConnect();
+}
+
 }  // namespace chrome_browser_net