Componentize component_updater: Move URLRequestPrepackagedInterceptor from content/ to net/

net/url_request/test_url_request_interceptor.cc

Index: net/url_request/test_url_request_interceptor.cc
diff --git a/net/url_request/test_url_request_interceptor.cc b/net/url_request/test_url_request_interceptor.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e9c340282d3b15f86f9ef872f2c1bc33258193f4
--- /dev/null
+++ b/net/url_request/test_url_request_interceptor.cc
@@ -0,0 +1,198 @@
+// 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 "net/url_request/test_url_request_interceptor.h"
+
+#include "base/file_util.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "base/threading/thread_restrictions.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_file_job.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_interceptor.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+class TestURLRequestJob : public net::URLRequestFileJob {
+ public:
+  TestURLRequestJob(net::URLRequest* request,
+                    net::NetworkDelegate* network_delegate,
+                    const base::FilePath& file_path,
+                    const scoped_refptr<base::TaskRunner>& worker_task_runner)
+      : net::URLRequestFileJob(request,
+                               network_delegate,
+                               file_path,
+                               worker_task_runner) {}
+
+  virtual int GetResponseCode() const OVERRIDE { return 200; }
+
+ private:
+  virtual ~TestURLRequestJob() {}
+
+  DISALLOW_COPY_AND_ASSIGN(TestURLRequestJob);
+};

[Ryan Sleevi, 2014/09/03 00:23:31]

Document?

To be quite honest, I'm not sure why this is needed (to force 200, even for a
missing file?)

[tommycli, 2014/09/05 15:03:08]

I looked into this. net::URLRequestFileJob always gives a -1 status code. So
this is required for the component updater to work correctly in tests.

I would also be okay with making net::URLRequestFileJob return a 200 or a 404
depending on missing or existing... though should we handle permissions on the
file too? 

+
+}  // namespace
+
+class TestURLRequestInterceptor::Delegate : public net::URLRequestInterceptor {

[Ryan Sleevi, 2014/09/03 00:23:31]

Briefly, document.

[tommycli, 2014/09/05 15:03:08]

Done.

+ public:
+  Delegate(const std::string& scheme,
+           const std::string& hostname,
+           const scoped_refptr<base::TaskRunner>& network_task_runner,
+           const scoped_refptr<base::TaskRunner>& worker_task_runner)
+      : scheme_(scheme),
+        hostname_(hostname),
+        network_task_runner_(network_task_runner),
+        worker_task_runner_(worker_task_runner),
+        hit_count_(0) {}
+  virtual ~Delegate() {}
+
+  void Register() {
+    net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
+        scheme_, hostname_, scoped_ptr<net::URLRequestInterceptor>(this));
+  }
+
+  static void Unregister(const std::string& scheme,
+                         const std::string& hostname) {
+    net::URLRequestFilter::GetInstance()->RemoveHostnameHandler(scheme,
+                                                                hostname);

[Ryan Sleevi, 2014/09/03 00:23:31]

Paul should review this, as it seems there's some asymmetry here in the API
calls.

[pauljensen, 2014/09/03 11:52:51]

This is WAI; seems fine.

[tommycli, 2014/09/05 15:03:08]

Done.

+  }
+
+  // When requests for |url| arrive, respond with the contents of |path|. The
+  // hostname and scheme of |url| must match the corresponding parameters
+  // passed as constructor arguments.
+  void SetResponse(const GURL& url,
+                   const base::FilePath& path,
+                   bool ignore_query) {
+    DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
+    // It's ok to do a blocking disk access on this thread; this class
+    // is just used for tests.

[Ryan Sleevi, 2014/09/03 00:23:31]

// Note: Actual loading of the file still occurs on |worker_task_runner_|

[tommycli, 2014/09/05 15:03:08]

I removed the ScopedAllowIO without any apparent bad effects.

+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+    EXPECT_TRUE(base::PathExists(path));

[Ryan Sleevi, 2014/09/03 00:23:31]

This EXPECT_TRUE is (largely) meaningless and won't propogate to any tests
unless they're doing ASSERT_NO_FATAL_FAILURE, which isn't documented anywhere as
part of the contract for this API.

[tommycli, 2014/09/05 15:03:08]

Done.

+    if (ignore_query) {
+      ignore_query_responses_[url] = path;
+    } else {
+      responses_[url] = path;
+    }
+  }
+
+  // Returns how many requests have been issued that have a stored reply.
+  int GetHitCount() const {
+    base::AutoLock auto_lock(hit_count_lock_);
+    return hit_count_;
+  }
+
+ private:
+  typedef std::map<GURL, base::FilePath> ResponseMap;
+
+  // When computing matches, this ignores the query parameters of the url.
+  virtual net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const OVERRIDE {
+    DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
+    if (request->url().scheme() != scheme_ ||
+        request->url().host() != hostname_) {
+      return NULL;
+    }
+
+    ResponseMap::const_iterator it = responses_.find(request->url());
+    if (it == responses_.end()) {
+      // Search for this request's url, ignoring any query parameters.
+      GURL url = request->url();
+      if (url.has_query()) {
+        GURL::Replacements replacements;
+        replacements.ClearQuery();
+        url = url.ReplaceComponents(replacements);
+      }
+      it = ignore_query_responses_.find(url);
+      if (it == ignore_query_responses_.end())
+        return NULL;
+    }
+    {
+      base::AutoLock auto_lock(hit_count_lock_);
+      ++hit_count_;
+    }
+
+    return new TestURLRequestJob(
+        request, network_delegate, it->second, worker_task_runner_);
+  }
+
+  const std::string scheme_;
+  const std::string hostname_;
+
+  const scoped_refptr<base::TaskRunner> network_task_runner_;
+  const scoped_refptr<base::TaskRunner> worker_task_runner_;
+
+  ResponseMap responses_;
+  ResponseMap ignore_query_responses_;
+
+  mutable base::Lock hit_count_lock_;
+  mutable int hit_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(Delegate);
+};
+
+TestURLRequestInterceptor::TestURLRequestInterceptor(
+    const std::string& scheme,
+    const std::string& hostname,
+    const scoped_refptr<base::TaskRunner>& network_task_runner,
+    const scoped_refptr<base::TaskRunner>& worker_task_runner)
+    : scheme_(scheme),
+      hostname_(hostname),
+      network_task_runner_(network_task_runner),
+      delegate_(new Delegate(scheme,
+                             hostname,
+                             network_task_runner_,
+                             worker_task_runner)) {
+  network_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&Delegate::Register, base::Unretained(delegate_)));
+}
+
+TestURLRequestInterceptor::~TestURLRequestInterceptor() {

[Ryan Sleevi, 2014/09/03 00:23:31]

BUG: This does not guarantee that |delegate_| will be safely cleaned up on the
network_task_runner, which means that if you create this object and delete it
without pumping the network task runner, you'll crash. You'll also run into
issues with line 166 and 179.

The solution is to DeleteSoon() the delegate from the network_task_runner, which
will then guarantee FIFO ordering for
Register/SetResponse/SetResponseIgnoreQuery calls, and then immediately delete
the object.

[tommycli, 2014/09/05 15:03:08]

I thought |delegate_| was simply never deleted here, but instead deleted by
net::URLRequestFilter::RemoveHostnameHandler when the Delagate::Unregister
method was called...

+  network_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&Delegate::Unregister, scheme_, hostname_));
+}
+
+void TestURLRequestInterceptor::SetResponse(const GURL& url,
+                                            const base::FilePath& path) {
+  CHECK_EQ(scheme_, url.scheme());
+  CHECK_EQ(hostname_, url.host());
+  network_task_runner_->PostTask(FROM_HERE,
+                                 base::Bind(&Delegate::SetResponse,
+                                            base::Unretained(delegate_),
+                                            url,
+                                            path,
+                                            false));
+}
+
+void TestURLRequestInterceptor::SetResponseIgnoreQuery(
+    const GURL& url,
+    const base::FilePath& path) {
+  CHECK_EQ(scheme_, url.scheme());
+  CHECK_EQ(hostname_, url.host());
+  network_task_runner_->PostTask(FROM_HERE,
+                                 base::Bind(&Delegate::SetResponse,
+                                            base::Unretained(delegate_),
+                                            url,
+                                            path,
+                                            true));
+}
+
+int TestURLRequestInterceptor::GetHitCount() {
+  return delegate_->GetHitCount();
+}
+
+LocalHostTestURLRequestInterceptor::LocalHostTestURLRequestInterceptor(
+    const scoped_refptr<base::TaskRunner>& network_task_runner,
+    const scoped_refptr<base::TaskRunner>& worker_task_runner)
+    : TestURLRequestInterceptor("http",
+                                "localhost",
+                                network_task_runner,
+                                worker_task_runner) {
+}
+
+}  // namespace net