First step to refactor ServiceWorkerVersion to make event dispatching more modular.

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 <map>
 #include <queue>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/containers/scoped_ptr_hash_map.h"
 #include "base/gtest_prod_util.h"
 #include "base/id_map.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/timer/timer.h"
 #include "content/browser/background_sync/background_sync_registration_handle.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/service_worker_script_cache_map.h"
 #include "content/common/background_sync_service.mojom.h"
 #include "content/common/content_export.h"
-#include "content/common/service_port_service.mojom.h"
 #include "content/common/service_worker/service_worker_status_code.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/common/service_registry.h"
 #include "third_party/WebKit/public/platform/WebGeofencingEventType.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerEventResult.h"
 
 // Windows headers will redefine SendMessage.
 #ifdef SendMessage
 #undef SendMessage
 #endif
@@ skipping 27 matching lines @@
 // one of them is activated. This class connects the actual script with a
 // running worker.
 class CONTENT_EXPORT ServiceWorkerVersion
     : NON_EXPORTED_BASE(public base::RefCounted<ServiceWorkerVersion>),
       public EmbeddedWorkerInstance::Listener {
  public:
   typedef base::Callback<void(ServiceWorkerStatusCode)> StatusCallback;
   typedef base::Callback<void(ServiceWorkerStatusCode,
                               ServiceWorkerFetchEventResult,
                               const ServiceWorkerResponse&)> FetchCallback;
-  typedef base::Callback<void(ServiceWorkerStatusCode,
-                              bool /* accept_connction */,
-                              const base::string16& /* name */,
-                              const base::string16& /* data */)>
-      ServicePortConnectCallback;
 
   enum RunningStatus {
     STOPPED = EmbeddedWorkerInstance::STOPPED,
     STARTING = EmbeddedWorkerInstance::STARTING,
     RUNNING = EmbeddedWorkerInstance::RUNNING,
     STOPPING = EmbeddedWorkerInstance::STOPPING,
   };
 
   // Current version status; some of the status (e.g. INSTALLED and ACTIVATED)
   // should be persisted unlike running status.
@@ skipping 80 matching lines @@
   // Schedules an update to be run 'soon'.
   void ScheduleUpdate();
 
   // If an update is scheduled but not yet started, this resets the timer
   // delaying the start time by a 'small' amount.
   void DeferScheduledUpdate();
 
   // Starts an update now.
   void StartUpdate();
 
+  // Starts the worker if it isn't already running, and calls |task| when the
+  // worker is running, or |error_callback| if starting the worker failed.
+  void RunAfterStartWorker(const StatusCallback& error_callback,
+                           const base::Closure& task);
+
+  // Call this while the worker is running before dispatching an event to the
+  // worker. This informs ServiceWorkerVersion about the event in progress.
+  // Returns a request id, which should later be passed to FinishRequest when
+  // the event finished.
+  // The |error_callback| is called if either ServiceWorkerVersion decides the
+  // event is taking too long, or if for some reason the worker stops or is
+  // killed before the request finishes.
+  int StartRequest(const StatusCallback& error_callback);
+
+  // Informs ServiceWorkerVersion that an event has finished being dispatched.
+  // Returns false if no pending requests with the provided id exist, for
+  // example if the request has already timed out.
+  bool FinishRequest(int request_id);
+
+  // Connects to a specific mojo service exposed by the (running) service
+  // worker. If a connection to a service for the same Interface already exists
+  // this will return that existing connection. The |request_id| must be a value
+  // previously returned by StartRequest. If the connection to the service
+  // fails or closes before the request finished, the error callback associated
+  // with |request_id| is called.
+  // Only call GetMojoServiceForRequest once for a specific |request_id|.
+  template <typename Interface>
+  base::WeakPtr<Interface> GetMojoServiceForRequest(int request_id);
+
   // Sends a message event to the associated embedded worker.
   void DispatchMessageEvent(
       const base::string16& message,
       const std::vector<TransferredMessagePort>& sent_message_ports,
       const StatusCallback& callback);
 
   // Sends install event to the associated embedded worker and asynchronously
   // calls |callback| when it errors out or it gets a response from the worker
   // to notify install completion.
   //
@@ skipping 56 matching lines @@
   // calls |callback| when it errors out or it gets a response from the worker
   // to notify completion.
   //
   // This must be called when the status() is ACTIVATED.
   void DispatchGeofencingEvent(
       const StatusCallback& callback,
       blink::WebGeofencingEventType event_type,
       const std::string& region_id,
       const blink::WebCircularGeofencingRegion& region);
 
-  // Sends a ServicePort connect event to the associated embedded worker and
-  // asynchronously calls |callback| with the response from the worker.
-  //
-  // This must be called when the status() is ACTIVATED.
-  void DispatchServicePortConnectEvent(
-      const ServicePortConnectCallback& callback,
-      const GURL& target_url,
-      const GURL& origin,
-      int port_id);
-
   // Sends a cross origin message event to the associated embedded worker and
   // asynchronously calls |callback| when the message was sent (or failed to
   // sent).
   // It is the responsibility of the code calling this method to make sure that
   // any transferred message ports are put on hold while potentially a process
   // for the service worker is spun up.
   //
   // This must be called when the status() is ACTIVATED.
   void DispatchCrossOriginMessageEvent(
       const NavigatorConnectClient& client,
@@ skipping 77 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, IdleTimeout);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, SetDevToolsAttached);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, StaleUpdate_FreshWorker);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest,
                            StaleUpdate_NonActiveWorker);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, StaleUpdate_StartWorker);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, StaleUpdate_RunningWorker);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest,
                            StaleUpdate_DoNotDeferTimer);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWaitForeverInFetchTest, RequestTimeout);
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, RequestTimeout);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerFailToStartTest, Timeout);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionBrowserTest,
                            TimeoutStartingWorker);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionBrowserTest,
                            TimeoutWorkerInEvent);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerStallInStoppingTest, DetachThenStart);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerStallInStoppingTest, DetachThenRestart);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest,
                            RegisterForeignFetchScopes);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, RequestCustomizedTimeout);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWaitForeverInFetchTest,
                            MixedRequestTimeouts);
 
   class Metrics;
   class PingController;
 
   // Used for UMA; add new entries to the end, before NUM_REQUEST_TYPES.
   enum RequestType {
     REQUEST_ACTIVATE,
     REQUEST_INSTALL,
     REQUEST_FETCH,
     REQUEST_SYNC,
     REQUEST_NOTIFICATION_CLICK,
     REQUEST_PUSH,
     REQUEST_GEOFENCING,
-    REQUEST_SERVICE_PORT_CONNECT,
+    REQUEST_CUSTOM,

[nhiroki, 2016/01/05 09:33:24]

I think it'd be better that each event has a unique type for UMA as is instead
of a general one. As you noted in your docs, how about passing the request type
into StartRequest()?

[Marijn Kruisselbrink, 2016/01/05 23:23:16]

Currently my approach was to add necessary functionality as I'm migrating
existing events. So given that fetch is currently the only event for which UMA
stats are collected by ServiceWorkerVersion I didn't yet add support for that. I
do still plan to add a ServiceWorkerMetrics::EventType parameter to
StartRequest, although I'm not quite sure yet how to deal with the potential
explosion of parameters to this method (especially where many of the parameters
should at least conceptually be optional. For background sync StartRequest will
need custom timeout behavior for example. But I don't really want to end up
with:
StartRequest(callback)
StartRequestWithEventType(callback, event_type)
StartRequestWithCustomTimeout(callback, timeout)
StartRequestWithEventTypeAndCustomTimeout(callback, event_type, timeout)
and who knows what other attributes eventually might apply to a request...
(or just having one overloaded method, or even one method with default
parameters, both of which aren't recommended by the styleguide).

So I figured I'd ignore this problem for now and deal with it when I actually
have to.

[nhiroki, 2016/01/06 10:13:57]

Thank you for the explanation.

On 2016/01/05 23:23:16, Marijn Kruisselbrink wrote:
You mean "ServiceWorker.EventHandledRatioType"? It's collected only for fetch
event, but there is other UMA stat collected for all event types
("ServiceWorker.RequestTimeouts.Count").
I think we may need to make |event_type| a required parameter to collect the
timeout stats. |timeout| would be optional.

[Marijn Kruisselbrink, 2016/01/07 00:59:08]

Ah yes, not sure how I missed the "Used for UMA" comment on RequestType. In that
case yes, this is something I need to deal with here already. In the latest
version I changed the code to no longer use RequestType for UMA stats, but
instead use ServiceWorkerMetrics::EventType (and I added all the existing values
from RequestType in the same order/with the same values to ensure histograms
keep being valid). That makes the existing code in SWVersion temporarily a bit
more complicated, as now all the event dispatching code has to specify both a
RequestType and an EventType, but in the future when all event dispatching is
done using StartRequest we should be able to completely get rid of RequestType
(and make various other simplifications as well).
As suggested, EventType now is also a required parameter to StartRequest, so
histograms should remain working the same. RecordEventHandledRatio still only
works (and is only called) for fetch events; I imagine if we'd want similar
histograms for other event types we could add some kind of event handled status
to FinishRequest, which would then collect the correct metrics.

     NUM_REQUEST_TYPES
   };
 
   struct RequestInfo {
     RequestInfo(int id, RequestType type, const base::TimeTicks& expiration);
     ~RequestInfo();
     bool operator>(const RequestInfo& other) const;
     int id;
     RequestType type;
     base::TimeTicks expiration;
   };
 
   template <typename CallbackType>
   struct PendingRequest {
     PendingRequest(const CallbackType& callback, const base::TimeTicks& time);
     ~PendingRequest();
 
     CallbackType callback;
     base::TimeTicks start_time;
+    // Name of the mojo service this request is associated with. Used to call
+    // the callback when a connection closes with outstanding requests.
+    // Compared as pointer, so should only contain static strings. Typically
+    // this would be Interface::Name_ for some mojo interface.
+    const char* mojo_service = nullptr;
+  };
+
+  // Base class to enable storing a list of mojo interface pointers for
+  // arbitrary interfaces. The destructor is also responsible for calling the
+  // error callbacks for any outstanding requests using this service.
+  class CONTENT_EXPORT BaseMojoService {
+   public:
+    BaseMojoService(ServiceWorkerVersion* worker, const char* service_name);
+    virtual ~BaseMojoService();
+
+   private:
+    ServiceWorkerVersion* worker_;
+    const char* service_name_;
+  };
+
+  // Wrapper around a mojo::InterfacePtr, which passes out WeakPtr's to the
+  // interface.
+  template <typename Interface>
+  class MojoService : public BaseMojoService {
+   public:
+    MojoService(ServiceWorkerVersion* worker,
+                mojo::InterfacePtr<Interface> interface)
+        : BaseMojoService(worker, Interface::Name_),
+          interface_(std::move(interface)),
+          weak_ptr_factory_(interface_.get()) {}
+
+    base::WeakPtr<Interface> GetWeakPtr() {
+      return weak_ptr_factory_.GetWeakPtr();
+    }
+
+   private:
+    mojo::InterfacePtr<Interface> interface_;
+    base::WeakPtrFactory<Interface> weak_ptr_factory_;
   };
 
   typedef ServiceWorkerVersion self;
   using ServiceWorkerClients = std::vector<ServiceWorkerClientInfo>;
   using RequestInfoPriorityQueue =
       std::priority_queue<RequestInfo,
                           std::vector<RequestInfo>,
                           std::greater<RequestInfo>>;
 
   // The timeout timer interval.
@@ skipping 50 matching lines @@
   void OnInstallEventFinished(int request_id,
                               blink::WebServiceWorkerEventResult result);
   void OnFetchEventFinished(int request_id,
                             ServiceWorkerFetchEventResult result,
                             const ServiceWorkerResponse& response);
   void OnSyncEventFinished(int request_id, ServiceWorkerEventStatus status);
   void OnNotificationClickEventFinished(int request_id);
   void OnPushEventFinished(int request_id,
                            blink::WebServiceWorkerEventResult result);
   void OnGeofencingEventFinished(int request_id);
-  void OnServicePortConnectEventFinished(int request_id,
-                                         ServicePortConnectResult result,
-                                         const mojo::String& name,
-                                         const mojo::String& data);
   void OnOpenWindow(int request_id, GURL url);
   void OnOpenWindowFinished(int request_id,
                             ServiceWorkerStatusCode status,
                             const std::string& client_uuid,
                             const ServiceWorkerClientInfo& client_info);
 
   void OnSetCachedMetadata(const GURL& url, const std::vector<char>& data);
   void OnSetCachedMetadataFinished(int64 callback_id, int result);
   void OnClearCachedMetadata(const GURL& url);
   void OnClearCachedMetadataFinished(int64 callback_id, int result);
@@ skipping 90 matching lines @@
 
   void FoundRegistrationForUpdate(
       ServiceWorkerStatusCode status,
       const scoped_refptr<ServiceWorkerRegistration>& registration);
 
   void OnStoppedInternal(EmbeddedWorkerInstance::Status old_status);
 
   // Called when a connection to a mojo event Dispatcher drops or fails.
   // Calls callbacks for any outstanding requests to the dispatcher as well
   // as cleans up the dispatcher.
-  void OnServicePortDispatcherConnectionError();
   void OnBackgroundSyncDispatcherConnectionError();
 
+  // Called when the remote side of a connection to a mojo service is lost.
+  void OnMojoConnectionError(const char* service_name);
+
   // Called at the beginning of each Dispatch*Event function: records
   // the time elapsed since idle (generally the time since the previous
   // event ended).
   void OnBeginEvent();
 
   const int64 version_id_;
   const int64 registration_id_;
   const GURL script_url_;
   const GURL scope_;
   std::vector<GURL> foreign_fetch_scopes_;
 
   Status status_ = NEW;
   scoped_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> activate_requests_;
   IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer> install_requests_;
   IDMap<PendingRequest<FetchCallback>, IDMapOwnPointer> fetch_requests_;
   IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer> sync_requests_;
   IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer>
       notification_click_requests_;
   IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer> push_requests_;
   IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer> geofencing_requests_;
-  IDMap<PendingRequest<ServicePortConnectCallback>, IDMapOwnPointer>
-      service_port_connect_requests_;
+  IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer> custom_requests_;
 
-  ServicePortDispatcherPtr service_port_dispatcher_;
   BackgroundSyncServiceClientPtr background_sync_dispatcher_;
 
+  // Stores all open connections to mojo services. Maps the service name to
+  // the actual interface pointer. When a connection is closed it is removed
+  // from this map.
+  // mojo_services_[Interface::Name_] is assumed to always contain a
+  // MojoService<Interface> instance.
+  base::ScopedPtrHashMap<const char*, scoped_ptr<BaseMojoService>>
+      mojo_services_;
+
   std::set<const ServiceWorkerURLRequestJob*> streaming_url_request_jobs_;
 
   std::map<std::string, ServiceWorkerProviderHost*> controllee_map_;
   // Will be null while shutting down.
   base::WeakPtr<ServiceWorkerContextCore> context_;
   base::ObserverList<Listener> listeners_;
   ServiceWorkerScriptCacheMap script_cache_map_;
   base::OneShotTimer update_timer_;
 
   // Starts running in StartWorker and continues until the worker is stopped.
@@ skipping 34 matching lines @@
 
   scoped_ptr<PingController> ping_controller_;
   scoped_ptr<Metrics> metrics_;
   const bool should_exclude_from_uma_ = false;
 
   base::WeakPtrFactory<ServiceWorkerVersion> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerVersion);
 };
 
+template <typename Interface>
+base::WeakPtr<Interface> ServiceWorkerVersion::GetMojoServiceForRequest(
+    int request_id) {
+  DCHECK_EQ(RUNNING, running_status());
+  PendingRequest<StatusCallback>* request = custom_requests_.Lookup(request_id);
+  DCHECK(request) << "Invalid request id";
+  DCHECK(!request->mojo_service)
+      << "Request is already associated with a mojo service";
+
+  MojoService<Interface>* service = static_cast<MojoService<Interface>*>(
+      mojo_services_.get(Interface::Name_));
+  if (!service) {
+    mojo::InterfacePtr<Interface> interface;
+    embedded_worker_->GetServiceRegistry()->ConnectToRemoteService(
+        mojo::GetProxy(&interface));
+    interface.set_connection_error_handler(
+        base::Bind(&ServiceWorkerVersion::OnMojoConnectionError,
+                   weak_factory_.GetWeakPtr(), Interface::Name_));
+    service = new MojoService<Interface>(this, std::move(interface));
+    mojo_services_.add(Interface::Name_, make_scoped_ptr(service));
+  }
+  request->mojo_service = Interface::Name_;
+  return service->GetWeakPtr();
+}
+
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_