Bug 1790207 - Forward declaration of mozilla::Result in nsIGlobalObject.h and other cleanup; r=dom-storage-reviewers,jesup

Depends on D162087

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

caps/nsJSPrincipals.h

@@ -35,11 +35,12 @@ class nsJSPrincipals : public nsIPrincipal, public JSPrincipals {
                                      uint32_t aTag,
                                      JSPrincipals** aOutPrincipals);
 
+  static bool ReadPrincipalInfo(JSStructuredCloneReader* aReader,
+                                mozilla::ipc::PrincipalInfo& aInfo);
+
   /* For write() implementations of off-main-thread JSPrincipals. */
   static bool WritePrincipalInfo(JSStructuredCloneWriter* aWriter,
                                  const mozilla::ipc::PrincipalInfo& aInfo);
-  static bool ReadPrincipalInfo(JSStructuredCloneReader* aReader,
-                                mozilla::ipc::PrincipalInfo& aInfo);
 
   // This class is used on the main thread to specify which principal to use
   // when reading principals data that was set on a DOM worker thread.

dom/base/nsIGlobalObject.cpp

@@ -6,17 +6,16 @@
 
 #include "nsIGlobalObject.h"
 #include "mozilla/CycleCollectedJSContext.h"
+#include "mozilla/Result.h"
 #include "mozilla/StorageAccess.h"
 #include "mozilla/dom/BlobURLProtocolHandler.h"
 #include "mozilla/dom/FunctionBinding.h"
-#include "mozilla/ipc/PBackgroundSharedTypes.h"
 #include "mozilla/dom/Report.h"
 #include "mozilla/dom/ReportingObserver.h"
 #include "mozilla/dom/ServiceWorker.h"
 #include "mozilla/dom/ServiceWorkerRegistration.h"
-#include "mozilla/dom/WorkerPrivate.h"
+#include "mozilla/ipc/PBackgroundSharedTypes.h"
 #include "nsContentUtils.h"
-#include "nsJSPrincipals.h"
 #include "nsThreadUtils.h"
 #include "nsGlobalWindowInner.h"
 
@@ -387,13 +386,13 @@ nsIGlobalObject::GetStorageKey() {
 }
 
 bool nsIGlobalObject::IsEqualStorageKey(
-    mozilla::ipc::PrincipalInfo& aPrincipalInfo) {
+    const mozilla::ipc::PrincipalInfo& aPrincipalInfo) {
   auto result = GetStorageKey();
-  mozilla::ipc::PrincipalInfo storagePrincipalInfo;
   if (result.isErr()) {
     return false;
   }
-  storagePrincipalInfo = result.unwrap();
+
+  const auto& storagePrincipalInfo = result.inspect();
 
   return mozilla::ipc::NonExpandedPrincipalInfoEquals(aPrincipalInfo,
                                                       storagePrincipalInfo);

dom/base/nsIGlobalObject.h

@@ -9,7 +9,6 @@
 
 #include "mozilla/LinkedList.h"
 #include "mozilla/Maybe.h"
-#include "mozilla/Result.h"
 #include "mozilla/dom/ClientInfo.h"
 #include "mozilla/dom/DispatcherTrait.h"
 #include "mozilla/dom/ServiceWorkerDescriptor.h"
@@ -35,6 +34,8 @@ class nsPIDOMWindowInner;
 
 namespace mozilla {
 class DOMEventTargetHelper;
+template <typename V, typename E>
+class Result;
 enum class StorageAccess;
 namespace dom {
 class VoidFunction;
@@ -48,6 +49,9 @@ class ServiceWorker;
 class ServiceWorkerRegistration;
 class ServiceWorkerRegistrationDescriptor;
 }  // namespace dom
+namespace ipc {
+class PrincipalInfo;
+}  // namespace ipc
 }  // namespace mozilla
 
 namespace JS::loader {
@@ -258,7 +262,7 @@ class nsIGlobalObject : public nsISupports,
 
   virtual mozilla::Result<mozilla::ipc::PrincipalInfo, nsresult>
   GetStorageKey();
-  virtual bool IsEqualStorageKey(mozilla::ipc::PrincipalInfo& aPrincipalInfo);
+  bool IsEqualStorageKey(const mozilla::ipc::PrincipalInfo& aPrincipalInfo);
 
  protected:
   virtual ~nsIGlobalObject();

dom/fs/api/FileSystemHandle.cpp

@@ -29,10 +29,10 @@ namespace mozilla::dom {
 
 namespace {
 
-bool ConstructHandleMetadata(JSContext* aCx, JSStructuredCloneReader* aReader,
-                             const bool aDirectory, nsIGlobalObject* aGlobal,
-                             fs::FileSystemEntryMetadata& aMetadata,
-                             mozilla::ipc::PrincipalInfo& info) {
+bool ConstructHandleMetadata(JSContext* aCx, nsIGlobalObject* aGlobal,
+                             JSStructuredCloneReader* aReader,
+                             const bool aDirectory,
+                             fs::FileSystemEntryMetadata& aMetadata) {
   using namespace mozilla::dom::fs;
 
   EntryId entryId;
@@ -54,19 +54,20 @@ bool ConstructHandleMetadata(JSContext* aCx, JSStructuredCloneReader* aReader,
     return false;
   }
 
-  aMetadata = fs::FileSystemEntryMetadata(entryId, name, aDirectory);
-
-  if (!nsJSPrincipals::ReadPrincipalInfo(aReader, info)) {
+  mozilla::ipc::PrincipalInfo principalInfo;
+  if (!nsJSPrincipals::ReadPrincipalInfo(aReader, principalInfo)) {
     return false;
   }
-  if (!aGlobal->IsEqualStorageKey(info)) {
+
+  if (!aGlobal->IsEqualStorageKey(principalInfo)) {
     LOG(("Blocking deserialization of %s due to cross-origin",
-         NS_ConvertUTF16toUTF8(aMetadata.entryName()).get()));
+         NS_ConvertUTF16toUTF8(name).get()));
     return false;
   }
-  LOG_VERBOSE(
-      ("Deserializing %s", NS_ConvertUTF16toUTF8(aMetadata.entryName()).get()));
 
+  LOG_VERBOSE(("Deserializing %s", NS_ConvertUTF16toUTF8(name).get()));
+
+  aMetadata = fs::FileSystemEntryMetadata(entryId, name, aDirectory);
   return true;
 }
 
@@ -257,8 +258,7 @@ bool FileSystemHandle::WriteStructuredClone(
 
   // Needed to make sure the destination nsIGlobalObject is from the same
   // origin/principal
-  QM_TRY_UNWRAP(mozilla::ipc::PrincipalInfo principalInfo,
-                mGlobal->GetStorageKey(), false);
+  QM_TRY_INSPECT(const auto& principalInfo, mGlobal->GetStorageKey(), false);
 
   return nsJSPrincipals::WritePrincipalInfo(aWriter, principalInfo);
 }
@@ -269,14 +269,14 @@ already_AddRefed<FileSystemFileHandle> FileSystemHandle::ConstructFileHandle(
     JSStructuredCloneReader* aReader) {
   using namespace mozilla::dom::fs;
 
-  mozilla::ipc::PrincipalInfo info;
   FileSystemEntryMetadata metadata;
-  if (!ConstructHandleMetadata(aCx, aReader, /* aDirectory */ false, aGlobal,
-                               metadata, info)) {
+  if (!ConstructHandleMetadata(aCx, aGlobal, aReader, /* aDirectory */ false,
+                               metadata)) {
     return nullptr;
   }
 
-  // Note that the actor may not be connected yet
+  // XXX Get the manager from Navigator!
+  // Note that the actor may not exist or may not be connected yet.
   auto fileSystemManager = MakeRefPtr<FileSystemManager>(aGlobal, nullptr);
 
   RefPtr<FileSystemFileHandle> fsHandle =
@@ -292,14 +292,14 @@ FileSystemHandle::ConstructDirectoryHandle(JSContext* aCx,
                                            JSStructuredCloneReader* aReader) {
   using namespace mozilla::dom::fs;
 
-  mozilla::ipc::PrincipalInfo info;
   FileSystemEntryMetadata metadata;
-  if (!ConstructHandleMetadata(aCx, aReader, /* aDirectory */ true, aGlobal,
-                               metadata, info)) {
+  if (!ConstructHandleMetadata(aCx, aGlobal, aReader, /* aDirectory */ true,
+                               metadata)) {
     return nullptr;
   }
-  // Note that the actor may not be connected yet
+
   // XXX Get the manager from Navigator!
+  // Note that the actor may not exist or may not be connected yet.
   auto fileSystemManager = MakeRefPtr<FileSystemManager>(aGlobal, nullptr);
 
   RefPtr<FileSystemDirectoryHandle> fsHandle =

dom/fs/api/FileSystemManager.cpp

@@ -12,12 +12,8 @@
 #include "mozilla/dom/FileSystemManagerChild.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/StorageManager.h"
-#include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/quota/QuotaCommon.h"
 #include "mozilla/dom/quota/ResultExtensions.h"
-#include "mozilla/ipc/BackgroundUtils.h"
-#include "mozilla/ipc/PBackgroundSharedTypes.h"
-#include "nsIScriptObjectPrincipal.h"
 
 namespace mozilla::dom {
 
@@ -67,9 +63,8 @@ void FileSystemManager::BeginRequest(
 
   MOZ_ASSERT(mGlobal);
 
-  QM_TRY_UNWRAP(mozilla::ipc::PrincipalInfo principalInfo,
-                mGlobal->GetStorageKey(), QM_VOID,
-                [aFailure](nsresult rv) { aFailure(rv); });
+  QM_TRY_INSPECT(const auto& principalInfo, mGlobal->GetStorageKey(), QM_VOID,
+                 [&aFailure](nsresult rv) { aFailure(rv); });
 
   mBackgroundRequestHandler->CreateFileSystemManagerChild(principalInfo)
       ->Then(

dom/fs/child/FileSystemRequestHandler.cpp

@@ -335,7 +335,7 @@ mozilla::ipc::RejectCallback GetRejectCallback(
 }
 
 struct BeginRequestFailureCallback {
-  BeginRequestFailureCallback(RefPtr<Promise> aPromise)
+  explicit BeginRequestFailureCallback(RefPtr<Promise> aPromise)
       : mPromise(std::move(aPromise)) {}
 
   void operator()(nsresult aRv) const {

dom/quota/ResultExtensions.h

@@ -134,12 +134,11 @@ Result<R, nsresult> ToResultGet(const Func& aFunc, Args&&... aArgs) {
 }  // namespace mozilla
 
 // TODO: Maybe move this to mfbt/ResultExtensions.h
-#define MOZ_TO_RESULT(expr) ::mozilla::ToResult(expr)
+#define MOZ_TO_RESULT(expr) ToResult(expr)
 
-#define QM_TO_RESULT(expr) ::mozilla::ToResult<QMResult>(expr)
+#define QM_TO_RESULT(expr) ToResult<QMResult>(expr)
 
-#define QM_TO_RESULT_TRANSFORM(value) \
-  ::mozilla::ToResultTransform<QMResult>(value)
+#define QM_TO_RESULT_TRANSFORM(value) ToResultTransform<QMResult>(value)
 
 #define MOZ_TO_RESULT_GET_TYPED(resultType, ...) \
   ::mozilla::ToResultGet<MOZ_REMOVE_PAREN(resultType)>(__VA_ARGS__)

dom/security/test/gtest/moz.build

@@ -16,8 +16,6 @@ if CONFIG["OS_TARGET"] != "Android":
         "TestUnexpectedPrivilegedLoads.cpp",
     ]
 
-include("/ipc/chromium/chromium-config.mozbuild")
-
 FINAL_LIBRARY = "xul-gtest"
 
 LOCAL_INCLUDES += [

dom/workers/WorkerScope.cpp

@@ -63,6 +63,7 @@
 #include "mozilla/dom/ImageBitmapSource.h"
 #include "mozilla/dom/MessagePortBinding.h"
 #include "mozilla/ipc/PBackgroundChild.h"
+#include "mozilla/ipc/PBackgroundSharedTypes.h"
 #include "mozilla/dom/Performance.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/PromiseWorkerProxy.h"
@@ -296,6 +297,24 @@ Maybe<ServiceWorkerDescriptor> WorkerGlobalScopeBase::GetController() const {
   return mClientSource->GetController();
 }
 
+mozilla::Result<mozilla::ipc::PrincipalInfo, nsresult>
+WorkerGlobalScopeBase::GetStorageKey() {
+  AssertIsOnWorkerThread();
+
+  const mozilla::ipc::PrincipalInfo& principalInfo =
+      mWorkerPrivate->GetEffectiveStoragePrincipalInfo();
+
+  // Block expanded and null principals, let content and system through.
+  if (principalInfo.type() !=
+          mozilla::ipc::PrincipalInfo::TContentPrincipalInfo &&
+      principalInfo.type() !=
+          mozilla::ipc::PrincipalInfo::TSystemPrincipalInfo) {
+    return Err(NS_ERROR_DOM_SECURITY_ERR);
+  }
+
+  return principalInfo;
+}
+
 void WorkerGlobalScopeBase::Control(
     const ServiceWorkerDescriptor& aServiceWorker) {
   AssertIsOnWorkerThread();
@@ -313,26 +332,6 @@ void WorkerGlobalScopeBase::Control(
   }
 }
 
-mozilla::Result<mozilla::ipc::PrincipalInfo, nsresult>
-WorkerGlobalScopeBase::GetStorageKey() {
-  using mozilla::ipc::PrincipalInfo;
-
-  MOZ_ASSERT(!NS_IsMainThread());
-
-  const PrincipalInfo& principalInfo =
-      mWorkerPrivate->GetEffectiveStoragePrincipalInfo();
-
-  // Block expanded and null principals, let content and system through.
-  if (principalInfo.type() !=
-          mozilla::ipc::PrincipalInfo::TContentPrincipalInfo &&
-      principalInfo.type() !=
-          mozilla::ipc::PrincipalInfo::TSystemPrincipalInfo) {
-    return Err(NS_ERROR_DOM_SECURITY_ERR);
-  }
-
-  return principalInfo;
-}
-
 nsresult WorkerGlobalScopeBase::Dispatch(
     TaskCategory aCategory, already_AddRefed<nsIRunnable>&& aRunnable) {
   return EventTargetFor(aCategory)->Dispatch(std::move(aRunnable),

dom/workers/WorkerScope.h

@@ -128,10 +128,9 @@ class WorkerGlobalScopeBase : public DOMEventTargetHelper,
 
   Maybe<ServiceWorkerDescriptor> GetController() const final;
 
-  virtual void Control(const ServiceWorkerDescriptor& aServiceWorker);
+  mozilla::Result<mozilla::ipc::PrincipalInfo, nsresult> GetStorageKey() final;
 
-  virtual mozilla::Result<mozilla::ipc::PrincipalInfo, nsresult> GetStorageKey()
-      override;
+  virtual void Control(const ServiceWorkerDescriptor& aServiceWorker);
 
   // DispatcherTrait implementation
   nsresult Dispatch(TaskCategory aCategory,

ipc/glue/BackgroundUtils.cpp

@@ -174,22 +174,22 @@ bool NonExpandedPrincipalInfoEquals(const PrincipalInfo& aLeft,
 
   const ContentPrincipalInfo& leftContent = aLeft.get_ContentPrincipalInfo();
   const ContentPrincipalInfo& rightContent = aRight.get_ContentPrincipalInfo();
+
   switch (aLeft.type()) {
-    case PrincipalInfo::TContentPrincipalInfo: {
+    case PrincipalInfo::TContentPrincipalInfo:
       return leftContent.attrs() == rightContent.attrs() &&
              leftContent.originNoSuffix() == rightContent.originNoSuffix();
-    }
-    case PrincipalInfo::TSystemPrincipalInfo: {
+
+    case PrincipalInfo::TSystemPrincipalInfo:
       // system principal always matches
       return true;
-    }
-    case PrincipalInfo::TNullPrincipalInfo: {
+
+    case PrincipalInfo::TNullPrincipalInfo:
       return leftContent.attrs() == rightContent.attrs() &&
              leftContent.spec() == rightContent.spec();
-    }
-    default: {
+
+    default:
       break;
-    }
   }
 
   // Clients (windows/workers) should never have an expanded principal type.

js/xpconnect/src/Sandbox.cpp

@@ -35,6 +35,7 @@
 #include "xpc_make_class.h"
 #include "XPCWrapper.h"
 #include "Crypto.h"
+#include "mozilla/Result.h"
 #include "mozilla/dom/AbortControllerBinding.h"
 #include "mozilla/dom/AutoEntryScript.h"
 #include "mozilla/dom/BindingCallContext.h"

js/xpconnect/src/XPCRuntimeService.cpp

@@ -9,6 +9,7 @@
 
 #include "nsContentUtils.h"
 #include "BackstagePass.h"
+#include "mozilla/Result.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/WebIDLGlobalNameHash.h"
 #include "mozilla/dom/IndexedDatabaseManager.h"