Eliminated the logic that accumulated multiple preconnect requests

chrome/browser/net/predictor_browsertest.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 "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"
 #include "chrome/common/pref_names.h"
 #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"
 
 using content::BrowserThread;
 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,
 // and immediately fails the resolution requests themselves.
 class HostResolutionRequestRecorder : public net::HostResolverProc {
  public:
   HostResolutionRequestRecorder()
       : HostResolverProc(NULL),
         is_waiting_for_hostname_(false) {
   }
@@ skipping 50 matching lines @@
   // requested and thus is running a nested message loop.
   bool is_waiting_for_hostname_;
 
   // A list of hostnames for which resolution has already been requested. Only
   // to be accessed from the UI thread.
   std::vector<std::string> requested_hostnames_;
 
   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 {
 
 class PredictorBrowserTest : public InProcessBrowserTest {
  public:
   PredictorBrowserTest()
       : startup_url_("http://host1:1"),
         referring_url_("http://host2:1"),
         target_url_("http://host3:1"),
         host_resolution_request_recorder_(new HostResolutionRequestRecorder) {
   }
 
  protected:
   void SetUpInProcessBrowserTestFixture() override {
     scoped_host_resolver_proc_.reset(new net::ScopedDefaultHostResolverProc(
         host_resolution_request_recorder_.get()));
     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();
   }
 
   void LearnAboutInitialNavigation(const GURL& url) {
     Predictor* predictor = browser()->profile()->GetNetworkPredictor();
     BrowserThread::PostTask(BrowserThread::IO,
                             FROM_HERE,
                             base::Bind(&Predictor::LearnAboutInitialNavigation,
@@ skipping 69 matching lines @@
 }
 
 IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, DnsPrefetch) {
   ASSERT_TRUE(test_server()->Start());
   ui_test_utils::NavigateToURL(
       browser(),
       GURL(test_server()->GetURL("files/predictor/dns_prefetch.html")));
   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