Bug 1584007 - let ClientChannelHelperParent manage FutureClientSourceParent lifetime r=dom-workers-and-storage-reviewers,mattwoodrow,asuth

ClientChannelHelperParent is the thing creating the ClientInfos which aren't
backed by existing ClientSources, so it may make sense for CCHP to tell the
ClientManagerService (CMS) to "expect" or "forget" a "future"
ClientSource(Parent).

When such a ClientInfo is created, CCHP notifies the CMS that a future
ClientSource may be created. This notification has to be observed before any
ClientHandles try to query CMS to a ClientSourceParent, which is the case
because the notification as well as ClientHandleParent constructors occur over
PBackground, and the notification sending method is called first.

CMS is told to forget the future ClientSource whenever a redirect occurs that
would result in the creation of a new ClientSource (i.e. a new ClientInfo). It's
also possible that the ClientInfo's LoadInfo's channel is cancelled. To account
for this, CHCP stores the most recent ClientInfo it's created and tells CMS
to _possibly_ forget the associated future ClientSource in its destructor. It's
possible that the channel completed its load, in which case this notification
is a no-op. This also relies on CHCP being destroyed after the reserved
ClientSource has a chance to both be created and register its
ClientSourceParent.

Differential Revision: https://phabricator.services.mozilla.com/D66529

dom/clients/manager/ClientChannelHelper.cpp

@@ -163,9 +163,9 @@ class ClientChannelHelper : public nsIInterfaceRequestor,
 
   NS_DECL_ISUPPORTS
 
-  static void CreateClientForPrincipal(nsILoadInfo* aLoadInfo,
-                                       nsIPrincipal* aPrincipal,
-                                       nsISerialEventTarget* aEventTarget) {
+  virtual void CreateClientForPrincipal(nsILoadInfo* aLoadInfo,
+                                        nsIPrincipal* aPrincipal,
+                                        nsISerialEventTarget* aEventTarget) {
     // Create the new ClientSource.  This should only happen for window
     // Clients since support cross-origin redirects are blocked by the
     // same-origin security policy.
@@ -181,16 +181,41 @@ NS_IMPL_ISUPPORTS(ClientChannelHelper, nsIInterfaceRequestor,
                   nsIChannelEventSink);
 
 class ClientChannelHelperParent final : public ClientChannelHelper {
-  ~ClientChannelHelperParent() = default;
+  ~ClientChannelHelperParent() {
+    // This requires that if a load completes, the associated ClientSource is
+    // created and registers itself before this ClientChannelHelperParent is
+    // destroyed. Otherwise, we may incorrectly "forget" a future ClientSource
+    // which will actually be created.
+    SetFutureSourceInfo(Nothing());
+  }
 
   void CreateClient(nsILoadInfo* aLoadInfo, nsIPrincipal* aPrincipal) override {
     CreateClientForPrincipal(aLoadInfo, aPrincipal, mEventTarget);
   }
 
+  void SetFutureSourceInfo(Maybe<ClientInfo>&& aClientInfo) {
+    if (mRecentFutureSourceInfo) {
+      // No-op if the corresponding ClientSource has alrady been created, but
+      // it's not known if that's the case here.
+      ClientManager::ForgetFutureSource(*mRecentFutureSourceInfo);
+    }
+
+    if (aClientInfo) {
+      Unused << NS_WARN_IF(!ClientManager::ExpectFutureSource(*aClientInfo));
+    }
+
+    mRecentFutureSourceInfo = std::move(aClientInfo);
+  }
+
+  // Keep track of the most recent ClientInfo created which isn't backed by a
+  // ClientSource, which is used to notify ClientManagerService that the
+  // ClientSource won't ever actually be constructed.
+  Maybe<ClientInfo> mRecentFutureSourceInfo;
+
  public:
-  static void CreateClientForPrincipal(nsILoadInfo* aLoadInfo,
-                                       nsIPrincipal* aPrincipal,
-                                       nsISerialEventTarget* aEventTarget) {
+  void CreateClientForPrincipal(nsILoadInfo* aLoadInfo,
+                                nsIPrincipal* aPrincipal,
+                                nsISerialEventTarget* aEventTarget) override {
     // If we're managing redirects in the parent, then we don't want
     // to create a new ClientSource (since those need to live with
     // the global), so just allocate a new ClientInfo/id and we can
@@ -200,6 +225,7 @@ class ClientChannelHelperParent final : public ClientChannelHelper {
         ClientManager::CreateInfo(ClientType::Window, aPrincipal);
     if (reservedInfo) {
       aLoadInfo->SetReservedClientInfo(*reservedInfo);
+      SetFutureSourceInfo(std::move(reservedInfo));
     }
   }
   ClientChannelHelperParent(nsIInterfaceRequestor* aOuter,
@@ -296,12 +322,12 @@ nsresult AddClientChannelHelperInternal(nsIChannel* aChannel,
   rv = aChannel->GetNotificationCallbacks(getter_AddRefs(outerCallbacks));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  if (initialClientInfo.isNothing() && reservedClientInfo.isNothing()) {
-    T::CreateClientForPrincipal(loadInfo, channelPrincipal, aEventTarget);
-  }
-
   RefPtr<ClientChannelHelper> helper = new T(outerCallbacks, aEventTarget);
 
+  if (initialClientInfo.isNothing() && reservedClientInfo.isNothing()) {
+    helper->CreateClientForPrincipal(loadInfo, channelPrincipal, aEventTarget);
+  }
+
   // Only set the callbacks helper if we are able to reserve the client
   // successfully.
   rv = aChannel->SetNotificationCallbacks(helper);