Bug 1517089 - Part 13: Use separate IPC params and response for datastore preloading; r=asuth
authorJan Varga <jan.varga@gmail.com>
Fri, 08 Feb 2019 21:02:11 +0100
changeset 520806 dc30924bd97b722614ad513e85e1d5be721991d5
parent 520805 a87a6189d20bdedd8ce554e9ec7f5334e690fc0a
child 520807 d430843fe84773ce381c0c9ad87cd62577305278
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1517089
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1517089 - Part 13: Use separate IPC params and response for datastore preloading; r=asuth Differential Revision: https://phabricator.services.mozilla.com/D19207
dom/ipc/ContentParent.cpp
dom/localstorage/ActorsParent.cpp
dom/localstorage/LSObject.cpp
dom/localstorage/LocalStorageManager2.cpp
dom/localstorage/PBackgroundLSRequest.ipdl
dom/localstorage/PBackgroundLSSharedTypes.ipdlh
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -5315,18 +5315,18 @@ nsresult ContentParent::AboutToLoadHttpF
     nsCOMPtr<nsILocalStorageManager> lsm =
         do_GetService("@mozilla.org/dom/localStorage-manager;1");
     if (NS_WARN_IF(!lsm)) {
       return NS_ERROR_FAILURE;
     }
 
     nsCOMPtr<nsISupports> dummy;
     rv = lsm->Preload(principal, nullptr, getter_AddRefs(dummy));
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Failed to preload local storage!");
     }
   }
 
   return NS_OK;
 }
 
 nsresult ContentParent::TransmitPermissionsForPrincipal(
     nsIPrincipal* aPrincipal) {
--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@@ -2223,29 +2223,30 @@ class PrepareDatastoreOp : public LSRequ
   RefPtr<PrepareDatastoreOp> mDelayedOp;
   RefPtr<DirectoryLock> mDirectoryLock;
   RefPtr<Connection> mConnection;
   RefPtr<Datastore> mDatastore;
   nsAutoPtr<ArchivedOriginScope> mArchivedOriginScope;
   LoadDataOp* mLoadDataOp;
   nsDataHashtable<nsStringHashKey, nsString> mValues;
   nsTArray<LSItemInfo> mOrderedItems;
-  const LSRequestPrepareDatastoreParams mParams;
+  const LSRequestCommonParams mParams;
   Maybe<ContentParentId> mContentParentId;
   nsCString mSuffix;
   nsCString mGroup;
   nsCString mMainThreadOrigin;
   nsCString mOrigin;
   nsString mDirectoryPath;
   nsString mDatabaseFilePath;
   uint32_t mPrivateBrowsingId;
   int64_t mUsage;
   int64_t mSizeOfKeys;
   int64_t mSizeOfItems;
   NestedState mNestedState;
+  const bool mCreateIfNotExists;
   bool mDatabaseNotAvailable;
   bool mRequestedDirectoryLock;
   bool mInvalidated;
 
 #ifdef DEBUG
   int64_t mDEBUGUsage;
 #endif
 
@@ -3125,19 +3126,30 @@ bool VerifyPrincipalInfo(const Principal
   return true;
 }
 
 bool VerifyRequestParams(const LSRequestParams& aParams) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aParams.type() != LSRequestParams::T__None);
 
   switch (aParams.type()) {
+    case LSRequestParams::TLSRequestPreloadDatastoreParams: {
+      const LSRequestCommonParams& params =
+          aParams.get_LSRequestPreloadDatastoreParams().commonParams();
+
+      if (NS_WARN_IF(!VerifyPrincipalInfo(params.principalInfo()))) {
+        ASSERT_UNLESS_FUZZING();
+        return false;
+      }
+      break;
+    }
+
     case LSRequestParams::TLSRequestPrepareDatastoreParams: {
-      const LSRequestPrepareDatastoreParams& params =
-          aParams.get_LSRequestPrepareDatastoreParams();
+      const LSRequestCommonParams& params =
+          aParams.get_LSRequestPrepareDatastoreParams().commonParams();
 
       if (NS_WARN_IF(!VerifyPrincipalInfo(params.principalInfo()))) {
         ASSERT_UNLESS_FUZZING();
         return false;
       }
       break;
     }
 
@@ -3185,16 +3197,17 @@ PBackgroundLSRequestParent* AllocPBackgr
   nsCOMPtr<nsIEventTarget> mainEventTarget;
   if (!BackgroundParent::IsOtherProcessActor(aBackgroundActor)) {
     mainEventTarget = LSObject::GetSyncLoopEventTarget();
   }
 
   RefPtr<LSRequestBase> actor;
 
   switch (aParams.type()) {
+    case LSRequestParams::TLSRequestPreloadDatastoreParams:
     case LSRequestParams::TLSRequestPrepareDatastoreParams: {
       Maybe<ContentParentId> contentParentId;
 
       uint64_t childID = BackgroundParent::GetChildID(aBackgroundActor);
       if (childID) {
         contentParentId = Some(ContentParentId(childID));
       }
 
@@ -5679,33 +5692,39 @@ mozilla::ipc::IPCResult LSRequestBase::R
  ******************************************************************************/
 
 PrepareDatastoreOp::PrepareDatastoreOp(
     nsIEventTarget* aMainEventTarget, const LSRequestParams& aParams,
     const Maybe<ContentParentId>& aContentParentId)
     : LSRequestBase(aMainEventTarget),
       mMainEventTarget(aMainEventTarget),
       mLoadDataOp(nullptr),
-      mParams(aParams.get_LSRequestPrepareDatastoreParams()),
+      mParams(
+          aParams.type() == LSRequestParams::TLSRequestPreloadDatastoreParams
+              ? aParams.get_LSRequestPreloadDatastoreParams().commonParams()
+              : aParams.get_LSRequestPrepareDatastoreParams().commonParams()),
       mContentParentId(aContentParentId),
       mPrivateBrowsingId(0),
       mUsage(0),
       mSizeOfKeys(0),
       mSizeOfItems(0),
       mNestedState(NestedState::BeforeNesting),
+      mCreateIfNotExists(aParams.type() ==
+                         LSRequestParams::TLSRequestPrepareDatastoreParams),
       mDatabaseNotAvailable(false),
       mRequestedDirectoryLock(false),
       mInvalidated(false)
 #ifdef DEBUG
       ,
       mDEBUGUsage(0)
 #endif
 {
-  MOZ_ASSERT(aParams.type() ==
-             LSRequestParams::TLSRequestPrepareDatastoreParams);
+  MOZ_ASSERT(
+      aParams.type() == LSRequestParams::TLSRequestPreloadDatastoreParams ||
+      aParams.type() == LSRequestParams::TLSRequestPrepareDatastoreParams);
 }
 
 PrepareDatastoreOp::~PrepareDatastoreOp() {
   MOZ_ASSERT(!mDirectoryLock);
   MOZ_ASSERT_IF(MayProceedOnNonOwningThread(),
                 mState == State::Initial || mState == State::Completed);
   MOZ_ASSERT(!mLoadDataOp);
 }
@@ -6011,17 +6030,17 @@ nsresult PrepareDatastoreOp::DatabaseWor
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     MOZ_ASSERT(gArchivedOrigins);
   }
 
   bool hasDataForMigration = mArchivedOriginScope->HasMatches(gArchivedOrigins);
 
-  bool createIfNotExists = mParams.createIfNotExists() || hasDataForMigration;
+  bool createIfNotExists = mCreateIfNotExists || hasDataForMigration;
 
   nsCOMPtr<nsIFile> directoryEntry;
   rv = quotaManager->EnsureOriginIsInitialized(
       PERSISTENCE_TYPE_DEFAULT, mSuffix, mGroup, mOrigin, createIfNotExists,
       getter_AddRefs(directoryEntry));
   if (rv == NS_ERROR_NOT_AVAILABLE) {
     return DatabaseNotAvailable();
   }
@@ -6457,22 +6476,21 @@ nsresult PrepareDatastoreOp::NestedRun()
 }
 
 void PrepareDatastoreOp::GetResponse(LSRequestResponse& aResponse) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mState == State::SendingResults);
   MOZ_ASSERT(NS_SUCCEEDED(ResultCode()));
 
   if (mDatabaseNotAvailable) {
-    MOZ_ASSERT(!mParams.createIfNotExists());
-
-    LSRequestPrepareDatastoreResponse prepareDatastoreResponse;
-    prepareDatastoreResponse.datastoreId() = null_t();
-
-    aResponse = prepareDatastoreResponse;
+    MOZ_ASSERT(!mCreateIfNotExists);
+
+    LSRequestPreloadDatastoreResponse preloadDatastoreResponse;
+
+    aResponse = preloadDatastoreResponse;
 
     return;
   }
 
   if (!mDatastore) {
     MOZ_ASSERT(mUsage == mDEBUGUsage);
 
     RefPtr<QuotaObject> quotaObject;
@@ -6496,33 +6514,39 @@ void PrepareDatastoreOp::GetResponse(LSR
     MOZ_ASSERT(!gDatastores->Get(mOrigin));
     gDatastores->Put(mOrigin, mDatastore);
   }
 
   uint64_t datastoreId = ++gLastDatastoreId;
 
   nsAutoPtr<PreparedDatastore> preparedDatastore(
       new PreparedDatastore(mDatastore, mContentParentId, mOrigin, datastoreId,
-                            /* aForPreload */ !mParams.createIfNotExists()));
+                            /* aForPreload */ !mCreateIfNotExists));
 
   if (!gPreparedDatastores) {
     gPreparedDatastores = new PreparedDatastoreHashtable();
   }
   gPreparedDatastores->Put(datastoreId, preparedDatastore);
 
   if (mInvalidated) {
     preparedDatastore->Invalidate();
   }
 
   preparedDatastore.forget();
 
-  LSRequestPrepareDatastoreResponse prepareDatastoreResponse;
-  prepareDatastoreResponse.datastoreId() = datastoreId;
-
-  aResponse = prepareDatastoreResponse;
+  if (mCreateIfNotExists) {
+    LSRequestPrepareDatastoreResponse prepareDatastoreResponse;
+    prepareDatastoreResponse.datastoreId() = datastoreId;
+
+    aResponse = prepareDatastoreResponse;
+  } else {
+    LSRequestPreloadDatastoreResponse preloadDatastoreResponse;
+
+    aResponse = preloadDatastoreResponse;
+  }
 }
 
 void PrepareDatastoreOp::Cleanup() {
   AssertIsOnOwningThread();
 
   if (mDatastore) {
     MOZ_ASSERT(!mDirectoryLock);
     MOZ_ASSERT(!mConnection);
--- a/dom/localstorage/LSObject.cpp
+++ b/dom/localstorage/LSObject.cpp
@@ -741,38 +741,36 @@ nsresult LSObject::EnsureDatabase() {
   // too late to initialize PBackground child on the owning thread, because
   // it can fail and parent would keep an extra strong ref to the datastore.
   PBackgroundChild* backgroundActor =
       BackgroundChild::GetOrCreateForCurrentThread();
   if (NS_WARN_IF(!backgroundActor)) {
     return NS_ERROR_FAILURE;
   }
 
-  LSRequestPrepareDatastoreParams params;
-  params.principalInfo() = *mPrincipalInfo;
-  params.originKey() = mOriginKey;
-  params.createIfNotExists() = true;
+  LSRequestCommonParams commonParams;
+  commonParams.principalInfo() = *mPrincipalInfo;
+  commonParams.originKey() = mOriginKey;
+
+  LSRequestPrepareDatastoreParams params(commonParams);
 
   LSRequestResponse response;
 
   nsresult rv = DoRequestSynchronously(params, response);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   MOZ_ASSERT(response.type() ==
              LSRequestResponse::TLSRequestPrepareDatastoreResponse);
 
   const LSRequestPrepareDatastoreResponse& prepareDatastoreResponse =
       response.get_LSRequestPrepareDatastoreResponse();
 
-  const NullableDatastoreId& datastoreId =
-      prepareDatastoreResponse.datastoreId();
-
-  MOZ_ASSERT(datastoreId.type() == NullableDatastoreId::Tuint64_t);
+  uint64_t datastoreId = prepareDatastoreResponse.datastoreId();
 
   // The datastore is now ready on the parent side (prepared by the asynchronous
   // request on the DOM File thread).
   // Let's create a direct connection to the datastore (through a database
   // actor) from the owning thread.
   // Note that we now can't error out, otherwise parent will keep an extra
   // strong reference to the datastore.
 
--- a/dom/localstorage/LocalStorageManager2.cpp
+++ b/dom/localstorage/LocalStorageManager2.cpp
@@ -22,17 +22,17 @@ class RequestResolver final : public LSR
 
   NS_INLINE_DECL_REFCOUNTING(mozilla::dom::RequestResolver, override);
 
  private:
   ~RequestResolver() = default;
 
   void HandleResponse(nsresult aResponse);
 
-  void HandleResponse(const NullableDatastoreId& aDatastoreId);
+  void HandleResponse();
 
   // LSRequestChildCallback
   void OnResponse(const LSRequestResponse& aResponse) override;
 };
 
 class SimpleRequestResolver final : public LSSimpleRequestChildCallback {
   RefPtr<Promise> mPromise;
 
@@ -187,34 +187,43 @@ LocalStorageManager2::GetNextGenLocalSto
 
 NS_IMETHODIMP
 LocalStorageManager2::Preload(nsIPrincipal* aPrincipal, JSContext* aContext,
                               nsISupports** _retval) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aPrincipal);
   MOZ_ASSERT(_retval);
 
-  nsresult rv;
+  nsCString originAttrSuffix;
+  nsCString originKey;
+  nsresult rv = GenerateOriginKey(aPrincipal, originAttrSuffix, originKey);
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  nsAutoPtr<PrincipalInfo> principalInfo(new PrincipalInfo());
+  rv = CheckedPrincipalToPrincipalInfo(aPrincipal, *principalInfo);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   RefPtr<Promise> promise;
 
   if (aContext) {
     rv = CreatePromise(aContext, getter_AddRefs(promise));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
-  LSRequestPrepareDatastoreParams params;
-  params.createIfNotExists() = false;
+  LSRequestCommonParams commonParams;
+  commonParams.principalInfo() = *principalInfo;
+  commonParams.originKey() = originKey;
 
-  rv = CheckedPrincipalToPrincipalInfo(aPrincipal, params.principalInfo());
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
+  LSRequestPreloadDatastoreParams params(commonParams);
 
   rv = StartRequest(promise, params);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   promise.forget(_retval);
   return NS_OK;
@@ -298,48 +307,36 @@ void RequestResolver::HandleResponse(nsr
 
   if (!mPromise) {
     return;
   }
 
   mPromise->MaybeReject(aResponse);
 }
 
-void RequestResolver::HandleResponse(const NullableDatastoreId& aDatastoreId) {
+void RequestResolver::HandleResponse() {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mPromise) {
     return;
   }
 
-  switch (aDatastoreId.type()) {
-    case NullableDatastoreId::Tnull_t:
-      mPromise->MaybeResolve(JS::NullHandleValue);
-      break;
-
-    case NullableDatastoreId::Tuint64_t:
-      mPromise->MaybeResolve(aDatastoreId.get_uint64_t());
-      break;
-
-    default:
-      MOZ_CRASH("Unknown datastore id type!");
-  }
+  mPromise->MaybeResolveWithUndefined();
 }
 
 void RequestResolver::OnResponse(const LSRequestResponse& aResponse) {
   MOZ_ASSERT(NS_IsMainThread());
 
   switch (aResponse.type()) {
     case LSRequestResponse::Tnsresult:
       HandleResponse(aResponse.get_nsresult());
       break;
 
-    case LSRequestResponse::TLSRequestPrepareDatastoreResponse:
-      HandleResponse(
-          aResponse.get_LSRequestPrepareDatastoreResponse().datastoreId());
+    case LSRequestResponse::TLSRequestPreloadDatastoreResponse:
+      HandleResponse();
       break;
     default:
       MOZ_CRASH("Unknown response type!");
   }
 }
 
 void SimpleRequestResolver::HandleResponse(nsresult aResponse) {
   MOZ_ASSERT(NS_IsMainThread());
--- a/dom/localstorage/PBackgroundLSRequest.ipdl
+++ b/dom/localstorage/PBackgroundLSRequest.ipdl
@@ -5,39 +5,38 @@
 include protocol PBackground;
 
 using struct mozilla::null_t
   from "ipc/IPCMessageUtils.h";
 
 namespace mozilla {
 namespace dom {
 
-union NullableDatastoreId
+struct LSRequestPreloadDatastoreResponse
 {
-  null_t;
-  uint64_t;
 };
 
 struct LSRequestPrepareDatastoreResponse
 {
-  NullableDatastoreId datastoreId;
+  uint64_t datastoreId;
 };
 
 struct LSRequestPrepareObserverResponse
 {
   uint64_t observerId;
 };
 
 /**
  * Discriminated union which can contain an error code (`nsresult`) or
  * particular request response.
  */
 union LSRequestResponse
 {
   nsresult;
+  LSRequestPreloadDatastoreResponse;
   LSRequestPrepareDatastoreResponse;
   LSRequestPrepareObserverResponse;
 };
 
 /**
  * An asynchronous protocol for issuing requests that are used in a synchronous
  * fashion by LocalStorage via LSObject's RequestHelper mechanism.  This differs
  * from LSSimpleRequest which is implemented and used asynchronously.
--- a/dom/localstorage/PBackgroundLSSharedTypes.ipdlh
+++ b/dom/localstorage/PBackgroundLSSharedTypes.ipdlh
@@ -2,30 +2,40 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 include PBackgroundSharedTypes;
 
 namespace mozilla {
 namespace dom {
 
-struct LSRequestPrepareDatastoreParams
+struct LSRequestCommonParams
 {
   PrincipalInfo principalInfo;
   nsCString originKey;
-  bool createIfNotExists;
+};
+
+struct LSRequestPreloadDatastoreParams
+{
+  LSRequestCommonParams commonParams;
+};
+
+struct LSRequestPrepareDatastoreParams
+{
+  LSRequestCommonParams commonParams;
 };
 
 struct LSRequestPrepareObserverParams
 {
   PrincipalInfo principalInfo;
 };
 
 union LSRequestParams
 {
+  LSRequestPreloadDatastoreParams;
   LSRequestPrepareDatastoreParams;
   LSRequestPrepareObserverParams;
 };
 
 struct LSSimpleRequestPreloadedParams
 {
   PrincipalInfo principalInfo;
 };