Limit timeout for foreign fetch events when triggered by another service worker.

content/browser/service_worker/foreign_fetch_request_handler_unittest.cc

Index: content/browser/service_worker/foreign_fetch_request_handler_unittest.cc
diff --git a/content/browser/service_worker/foreign_fetch_request_handler_unittest.cc b/content/browser/service_worker/foreign_fetch_request_handler_unittest.cc
index 8a12374b6417f8a65a94d6a9a04151cba6117b12..f52dc428dec1ffcf11a7aebde260c5568fa5c04f 100644
--- a/content/browser/service_worker/foreign_fetch_request_handler_unittest.cc
+++ b/content/browser/service_worker/foreign_fetch_request_handler_unittest.cc
@@ -6,18 +6,24 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "content/browser/browser_thread_impl.h"
+#include "content/browser/fileapi/mock_url_request_delegate.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_test_utils.h"
 #include "content/browser/service_worker/service_worker_version.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/origin_trial_policy.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "content/test/test_content_browser_client.h"
 #include "net/http/http_response_info.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/test_data_directory.h"
+#include "net/url_request/url_request_context.h"
+#include "storage/browser/blob/blob_storage_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
@@ -39,6 +45,12 @@ const uint8_t kTestPublicKey[] = {
     0x64, 0x90, 0x08, 0x8e, 0xa8, 0xe0, 0x56, 0x3a, 0x04, 0xd0,
 };
 
+int kMockProviderId = 1;
+
+const char* kValidUrl = "https://valid.example.com/foo/bar";
+
+void EmptyCallback() {}
+
 }  // namespace
 
 class ForeignFetchRequestHandlerTest : public testing::Test {
@@ -46,6 +58,7 @@ class ForeignFetchRequestHandlerTest : public testing::Test {
   ForeignFetchRequestHandlerTest()
       : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {
     SetContentClient(&test_content_client_);
+    SetBrowserClientForTesting(&test_content_browser_client_);
   }
   ~ForeignFetchRequestHandlerTest() override {}
 
@@ -59,6 +72,35 @@ class ForeignFetchRequestHandlerTest : public testing::Test {
                                                   context()->AsWeakPtr());
     version_ = new ServiceWorkerVersion(registration_.get(), kResource1,
                                         kVersionId, context()->AsWeakPtr());
+
+    version_->set_foreign_fetch_scopes({kScope});
+
+    // An empty host.
+    std::unique_ptr<ServiceWorkerProviderHost> host(
+        new ServiceWorkerProviderHost(
+            helper_->mock_render_process_id(), MSG_ROUTING_NONE,
+            kMockProviderId, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
+            ServiceWorkerProviderHost::FrameSecurityLevel::SECURE,
+            context()->AsWeakPtr(), nullptr));
+    host->SetDocumentUrl(GURL("https://host/scope/"));
+    provider_host_ = host->AsWeakPtr();
+    context()->AddProviderHost(std::move(host));
+
+    context()->storage()->LazyInitialize(base::Bind(&EmptyCallback));
+    base::RunLoop().RunUntilIdle();
+
+    std::vector<ServiceWorkerDatabase::ResourceRecord> records;
+    records.push_back(
+        ServiceWorkerDatabase::ResourceRecord(10, version_->script_url(), 100));
+    version_->script_cache_map()->SetResources(records);
+    version_->set_fetch_handler_existence(
+        ServiceWorkerVersion::FetchHandlerExistence::EXISTS);
+    version_->SetStatus(ServiceWorkerVersion::ACTIVATED);
+    registration_->SetActiveVersion(version_);
+    context()->storage()->StoreRegistration(
+        registration_.get(), version_.get(),
+        base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
+    base::RunLoop().RunUntilIdle();
   }
 
   void TearDown() override {
@@ -70,11 +112,22 @@ class ForeignFetchRequestHandlerTest : public testing::Test {
 
  protected:
   ServiceWorkerContextCore* context() const { return helper_->context(); }
+  ServiceWorkerContextWrapper* context_wrapper() const {
+    return helper_->context_wrapper();
+  }
+  ServiceWorkerProviderHost* provider_host() const {
+    return provider_host_.get();
+  }
 
   bool CheckOriginTrialToken(const ServiceWorkerVersion* const version) const {
     return ForeignFetchRequestHandler::CheckOriginTrialToken(version);
   }
 
+  base::Optional<base::TimeDelta> timeout_for_request(
+      ForeignFetchRequestHandler* handler) {
+    return handler->timeout_;
+  }
+
   ServiceWorkerVersion* version() const { return version_.get(); }
 
   static std::unique_ptr<net::HttpResponseInfo> CreateTestHttpResponseInfo() {
@@ -90,6 +143,59 @@ class ForeignFetchRequestHandlerTest : public testing::Test {
     return http_info;
   }
 
+  ForeignFetchRequestHandler* InitializeHandler(const std::string& url,
+                                                ResourceType resource_type,
+                                                const char* initiator) {
+    request_ = url_request_context_.CreateRequest(
+        GURL(url), net::DEFAULT_PRIORITY, &url_request_delegate_);
+    if (initiator)
+      request_->set_initiator(url::Origin(GURL(initiator)));
+    ForeignFetchRequestHandler::InitializeHandler(
+        request_.get(), context_wrapper(), &blob_storage_context_,
+        helper_->mock_render_process_id(), kMockProviderId,
+        SkipServiceWorker::NONE, FETCH_REQUEST_MODE_CORS,
+        FETCH_CREDENTIALS_MODE_OMIT, FetchRedirectMode::FOLLOW_MODE,
+        resource_type, REQUEST_CONTEXT_TYPE_FETCH,
+        REQUEST_CONTEXT_FRAME_TYPE_NONE, nullptr,
+        true /* initiated_in_secure_context */);
+
+    return ForeignFetchRequestHandler::GetHandler(request_.get());
+  }
+
+  void CreateServiceWorkerTypeProviderHost() {
+    std::unique_ptr<ServiceWorkerProviderHost> host(
+        new ServiceWorkerProviderHost(
+            helper_->mock_render_process_id(), MSG_ROUTING_NONE,
+            kMockProviderId, SERVICE_WORKER_PROVIDER_FOR_CONTROLLER,
+            ServiceWorkerProviderHost::FrameSecurityLevel::UNINITIALIZED,
+            context()->AsWeakPtr(), nullptr));
+    provider_host_ = host->AsWeakPtr();
+    context()->RemoveProviderHost(host->process_id(), host->provider_id());
+    context()->AddProviderHost(std::move(host));
+
+    scoped_refptr<ServiceWorkerRegistration> registration =
+        new ServiceWorkerRegistration(GURL("https://host/scope"), 1L,
+                                      context()->AsWeakPtr());
+    scoped_refptr<ServiceWorkerVersion> version = new ServiceWorkerVersion(
+        registration.get(), GURL("https://host/script.js"), 1L,
+        context()->AsWeakPtr());
+
+    std::vector<ServiceWorkerDatabase::ResourceRecord> records;
+    records.push_back(
+        ServiceWorkerDatabase::ResourceRecord(10, version->script_url(), 100));
+    version->script_cache_map()->SetResources(records);
+    version->set_fetch_handler_existence(
+        ServiceWorkerVersion::FetchHandlerExistence::EXISTS);
+    version->SetStatus(ServiceWorkerVersion::ACTIVATED);
+    registration->SetActiveVersion(version);
+    context()->storage()->StoreRegistration(
+        registration.get(), version.get(),
+        base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
+    base::RunLoop().RunUntilIdle();
+
+    provider_host_->running_hosted_version_ = version;
+  }
+
  private:
   class TestContentClient : public ContentClient {
    public:
@@ -116,9 +222,16 @@ class ForeignFetchRequestHandlerTest : public testing::Test {
   scoped_refptr<ServiceWorkerRegistration> registration_;
   scoped_refptr<ServiceWorkerVersion> version_;
   TestContentClient test_content_client_;
+  TestContentBrowserClient test_content_browser_client_;
   std::unique_ptr<EmbeddedWorkerTestHelper> helper_;
   TestBrowserThreadBundle browser_thread_bundle_;
 
+  net::URLRequestContext url_request_context_;
+  MockURLRequestDelegate url_request_delegate_;
+  base::WeakPtr<ServiceWorkerProviderHost> provider_host_;
+  storage::BlobStorageContext blob_storage_context_;
+  std::unique_ptr<net::URLRequest> request_;
+
   DISALLOW_COPY_AND_ASSIGN(ForeignFetchRequestHandlerTest);
 };
 
@@ -185,4 +298,69 @@ TEST_F(ForeignFetchRequestHandlerTest, CheckOriginTrialToken_ExpiredToken) {
   EXPECT_FALSE(CheckOriginTrialToken(version()));
 }
 
+TEST_F(ForeignFetchRequestHandlerTest, InitializeHandler_Success) {
+  EXPECT_TRUE(InitializeHandler(kValidUrl, RESOURCE_TYPE_IMAGE,
+                                nullptr /* initiator */));
+}
+
+TEST_F(ForeignFetchRequestHandlerTest, InitializeHandler_WrongResourceType) {
+  EXPECT_FALSE(InitializeHandler(kValidUrl, RESOURCE_TYPE_MAIN_FRAME,
+                                 nullptr /* initiator */));
+  EXPECT_FALSE(InitializeHandler(kValidUrl, RESOURCE_TYPE_SUB_FRAME,
+                                 nullptr /* initiator */));
+  // EXPECT_FALSE(InitializeHandler(kValidUrl, RESOURCE_TYPE_WORKER, nullptr /*

[Marijn Kruisselbrink, 2016/12/02 00:15:02]

It surprised me a lot that ServiceWorkerUtils::IsMainResourceType does not
return true for dedicated workers or service workers. For whatever reason it
only considers shared workers to be main resources. Is that intentional? If so,
I guess I'll have to change my usage of it.

[falken, 2016/12/05 04:34:48]

I think the reason is that IsMainResourceType really means: "this provider
should have a controller". SW controlling a dedicated worker doesn't look fully
supported yet, we don't actually create a provider host with TYPE_WORKER ever
(https://codereview.chromium.org/717553003) or expose navigator.serviceWorker
(https://bugs.chromium.org/p/chromium/issues/detail?id=371690). It looks like
the SW does intercept fetches from the dedicated worker though, probably loads
from the dedicated worker are plumbed to its owning page.

Service workers are never controlled by another service worker by design.

So I think as of now we still expect InitializeHandler to return false for
non-shared worker, non-frame. Would certainly be better to have comments
somewhere explaining this.

[Marijn Kruisselbrink, 2016/12/05 20:46:52]

Hmm, okay... It also somewhat looks like this IsMainResourceType is just a copy
of the same method in AppCache code. Anyway, I most certainly never intended for
foreign fetch to be able to intercept main resources for workers, so for now
I've just changed the check in ForeignFetchRequestHandler to explicitly not
handle those resource types. But it seems like it would make sense to rename
IsMainResourceType to not imply that it returns true for all main resource
types...

+  // initiator */));
+  EXPECT_FALSE(InitializeHandler(kValidUrl, RESOURCE_TYPE_SHARED_WORKER,
+                                 nullptr /* initiator */));
+  // EXPECT_FALSE(InitializeHandler(kValidUrl, RESOURCE_TYPE_SERVICE_WORKER,
+  // nullptr /* initiator */));
+}
+
+TEST_F(ForeignFetchRequestHandlerTest, InitializeHandler_SameOriginRequest) {
+  EXPECT_FALSE(InitializeHandler(kValidUrl, RESOURCE_TYPE_IMAGE,
+                                 kValidUrl /* initiator */));
+}
+
+TEST_F(ForeignFetchRequestHandlerTest, InitializeHandler_NoRegisteredHandlers) {
+  EXPECT_FALSE(InitializeHandler("https://invalid.example.com/foo",
+                                 RESOURCE_TYPE_IMAGE, nullptr /* initiator */));
+}
+
+TEST_F(ForeignFetchRequestHandlerTest, InitializeHandler_TimeoutBehavior) {
+  ForeignFetchRequestHandler* handler =
+      InitializeHandler("https://valid.example.com/foo", RESOURCE_TYPE_IMAGE,
+                        nullptr /* initiator */);
+  ASSERT_TRUE(handler);
+
+  EXPECT_EQ(base::nullopt, timeout_for_request(handler));
+
+  CreateServiceWorkerTypeProviderHost();
+  ServiceWorkerVersion* version = provider_host()->running_hosted_version();
+
+  // Set mock clock on version to check timeout behavior.
+  base::SimpleTestTickClock* tick_clock = new base::SimpleTestTickClock();
+  tick_clock->SetNowTicks(base::TimeTicks::Now());
+  version->SetTickClockForTesting(base::WrapUnique(tick_clock));
+
+  // Make sure worker has a non-zero timeout.
+  version->StartWorker(ServiceWorkerMetrics::EventType::UNKNOWN,
+                       base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
+  base::RunLoop().RunUntilIdle();
+  version->StartRequestWithCustomTimeout(
+      ServiceWorkerMetrics::EventType::ACTIVATE,
+      base::Bind(&ServiceWorkerUtils::NoOpStatusCallback),
+      base::TimeDelta::FromSeconds(10), ServiceWorkerVersion::KILL_ON_TIMEOUT);
+
+  // Advance clock by a couple seconds.
+  tick_clock->Advance(base::TimeDelta::FromSeconds(4));
+  base::TimeDelta remaining_time = version->remaining_timeout();
+  EXPECT_EQ(base::TimeDelta::FromSeconds(6), remaining_time);
+
+  // Make sure new request only gets remaining timeout.
+  handler = InitializeHandler("https://valid.example.com/foo",
+                              RESOURCE_TYPE_IMAGE, nullptr /* initiator */);
+  ASSERT_TRUE(handler);
+  EXPECT_EQ(remaining_time, timeout_for_request(handler));
+}
+
 }  // namespace content