ServiceWorker: Store the existence of fetch event handler

content/browser/service_worker/service_worker_version.h

 // Copyright 2013 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.
 
 #ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 
 #include <stdint.h>
 
 #include <functional>
@@ skipping 119 matching lines @@
 
   int64_t version_id() const { return version_id_; }
   int64_t registration_id() const { return registration_id_; }
   const GURL& script_url() const { return script_url_; }
   const GURL& scope() const { return scope_; }
   RunningStatus running_status() const {
     return static_cast<RunningStatus>(embedded_worker_->status());
   }
   ServiceWorkerVersionInfo GetInfo();
   Status status() const { return status_; }
+  bool has_fetch_handler() const { return has_fetch_handler_; }
+  void set_has_fetch_handler(bool has_fetch_handler) {
+    has_fetch_handler_ = has_fetch_handler;
+  }
 
   const std::vector<GURL>& foreign_fetch_scopes() const {
     return foreign_fetch_scopes_;
   }
   void set_foreign_fetch_scopes(const std::vector<GURL>& scopes) {
     foreign_fetch_scopes_ = scopes;
   }
 
   const std::vector<url::Origin>& foreign_fetch_origins() const {
     return foreign_fetch_origins_;
@@ skipping 91 matching lines @@
 
   // For simple events where the full functionality of DispatchEvent is not
   // needed, this method can be used instead. The ResponseMessage must consist
   // of just a request_id and a blink::WebServiceWorkerEventResult field. The
   // result is converted to a ServiceWorkerStatusCode and passed to the error
   // handler associated with the request. Additionally this methods calls
   // FinishRequest before passing the reply to the callback.
   template <typename ResponseMessage>
   void DispatchSimpleEvent(int request_id, const IPC::Message& message);
 
+  // For install event, the callback should be specialized to update the stat
+  // of version object because |has_fetch_handler| should be set corresponding
+  // to the result of script evaluation.

[falken, 2016/05/23 08:41:13]

I think DispatchEvent() (not DispatchSimpleEvent) was designed to handle
specialized callbacks. Can you use that instead of special casing Install? (It
looks like currently only "fetch" uses it.)

That would move handling the has_fetch_handler flag to ServiceWorkerRegisterJob,
which feels a bit cleaner, too.

[shimazu, 2016/05/23 10:44:24]

I considered has_fetch_handler is a property which should be bound to
ServiceWorkerVersion, so that is updated only by ServiceWorkerVersion itself or
ServiceWorkerStorage in this patch.

What do you think?

[falken, 2016/05/24 01:15:34]

But SWVersion is kind of transient: it can be destroyed and recreated for the
same worker. This patch is depending on the version getting persisted
immediately after setting the bit. We know that is what happens, but I'd rather
make that dependency explicit. The current patch does:
1. SWRegisterJob dispatches install event.
2. SWVersion sets the bit on itself.
3. SWRegisterJob gets the bit and persists the worker.

Alternatively, this seems clearer:
1. SWRegisterJob dispatches the install event.
2. SWRegisterJob install event callback is passed the bit and persists the
worker.

But more than that, the special casing of DispatchInstallEvent seems unnecessary
since we have DispatchEvent.

[shimazu, 2016/05/24 04:17:46]

Thanks. Done.

+  void DispatchInstallEvent(int request_id);
+
   // Adds and removes |provider_host| as a controllee of this ServiceWorker.
   // A potential controllee is a host having the version as its .installing
   // or .waiting version.
   void AddControllee(ServiceWorkerProviderHost* provider_host);
   void RemoveControllee(ServiceWorkerProviderHost* provider_host);
 
   // Returns if it has controllee.
   bool HasControllee() const { return !controllee_map_.empty(); }
   std::map<std::string, ServiceWorkerProviderHost*> controllee_map() {
     return controllee_map_;
@@ skipping 232 matching lines @@
 
   // This corresponds to the spec's get(id) steps.
   void OnGetClient(int request_id, const std::string& client_uuid);
 
   // This corresponds to the spec's matchAll(options) steps.
   void OnGetClients(int request_id,
                     const ServiceWorkerClientQueryOptions& options);
 
   void OnSimpleEventResponse(int request_id,
                              blink::WebServiceWorkerEventResult result);
+  void OnInstallEventResponse(int request_id,
+                              blink::WebServiceWorkerEventResult result,
+                              bool has_fetch_handler);
   void OnOpenWindow(int request_id, GURL url);
   void OnOpenWindowFinished(int request_id,
                             ServiceWorkerStatusCode status,
                             const ServiceWorkerClientInfo& client_info);
 
   void OnSetCachedMetadata(const GURL& url, const std::vector<char>& data);
   void OnSetCachedMetadataFinished(int64_t callback_id, int result);
   void OnClearCachedMetadata(const GURL& url);
   void OnClearCachedMetadataFinished(int64_t callback_id, int result);
 
@@ skipping 83 matching lines @@
   // the time elapsed since idle (generally the time since the previous
   // event ended).
   void OnBeginEvent();
 
   const int64_t version_id_;
   const int64_t registration_id_;
   const GURL script_url_;
   const GURL scope_;
   std::vector<GURL> foreign_fetch_scopes_;
   std::vector<url::Origin> foreign_fetch_origins_;
+  bool has_fetch_handler_ = true;
 
   Status status_ = NEW;
   std::unique_ptr<EmbeddedWorkerInstance> embedded_worker_;
   std::vector<StatusCallback> start_callbacks_;
   std::vector<StatusCallback> stop_callbacks_;
   std::vector<base::Closure> status_change_callbacks_;
 
   // Message callbacks. (Update HasInflightRequests() too when you update this
   // list.)
   IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer> custom_requests_;
@@ skipping 135 matching lines @@
 
   // At this point |this| can have been deleted, so don't do anything other
   // than returning.
 
   return true;
 }
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_