Bug 1507479: Make WorkerPrivate::mIsSecureContext const r=perry,asuth
authorYaron Tausky <ytausky@mozilla.com>
Wed, 21 Nov 2018 20:02:51 +0000
changeset 504083 81a99e4b008c5edf36d54d12d8ef9dd430e4c81c
parent 504082 b1c8590d5f74c3e4ca3daf8caaf0662e8d126985
child 504084 f111ecab40ad2216074d878b21125fd5d4994994
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersperry, asuth
bugs1507479
milestone65.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 1507479: Make WorkerPrivate::mIsSecureContext const r=perry,asuth This member was never changed after it was set, but it couldn't be const because WorkerLoadInfo used a custom StealFrom() function instead of an idiomatic move constructor and move assignment operator. This commit replaces StealFrom with its idiomatic counterparts, adds a function that determines a new worker's secure context status, and uses that function to initialize the now const mIsSecureContext. Making constant members const is part of a greater effort to ensure that data is either modified from a single thread only, or access to it is synchronized properly. Differential Revision: https://phabricator.services.mozilla.com/D12030
dom/workers/WorkerLoadInfo.cpp
dom/workers/WorkerLoadInfo.h
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
--- a/dom/workers/WorkerLoadInfo.cpp
+++ b/dom/workers/WorkerLoadInfo.cpp
@@ -79,94 +79,30 @@ SwapToISupportsArray(SmartPtr<T>& aSrc,
 
   nsISupports* rawSupports =
     static_cast<typename ISupportsBaseInfo<T>::ISupportsBase*>(raw);
   dest->swap(rawSupports);
 }
 
 } // anonymous
 
-WorkerLoadInfo::WorkerLoadInfo()
+WorkerLoadInfoData::WorkerLoadInfoData()
   : mLoadFlags(nsIRequest::LOAD_NORMAL)
   , mWindowID(UINT64_MAX)
   , mReferrerPolicy(net::RP_Unset)
   , mFromWindow(false)
   , mEvalAllowed(false)
   , mReportCSPViolations(false)
   , mXHRParamsAllowed(false)
   , mPrincipalIsSystem(false)
   , mStorageAllowed(false)
   , mFirstPartyStorageAccessGranted(false)
   , mServiceWorkersTestingInWindow(false)
   , mSecureContext(eNotSet)
-{
-  MOZ_COUNT_CTOR(WorkerLoadInfo);
-}
-
-WorkerLoadInfo::~WorkerLoadInfo()
-{
-  MOZ_COUNT_DTOR(WorkerLoadInfo);
-}
-
-void
-WorkerLoadInfo::StealFrom(WorkerLoadInfo& aOther)
-{
-  MOZ_ASSERT(!mBaseURI);
-  aOther.mBaseURI.swap(mBaseURI);
-
-  MOZ_ASSERT(!mResolvedScriptURI);
-  aOther.mResolvedScriptURI.swap(mResolvedScriptURI);
-
-  MOZ_ASSERT(!mPrincipal);
-  aOther.mPrincipal.swap(mPrincipal);
-
-  // mLoadingPrincipal can be null if this is a ServiceWorker.
-  aOther.mLoadingPrincipal.swap(mLoadingPrincipal);
-
-  MOZ_ASSERT(!mScriptContext);
-  aOther.mScriptContext.swap(mScriptContext);
-
-  MOZ_ASSERT(!mWindow);
-  aOther.mWindow.swap(mWindow);
-
-  MOZ_ASSERT(!mCSP);
-  aOther.mCSP.swap(mCSP);
-
-  MOZ_ASSERT(!mChannel);
-  aOther.mChannel.swap(mChannel);
-
-  MOZ_ASSERT(!mLoadGroup);
-  aOther.mLoadGroup.swap(mLoadGroup);
-
-  MOZ_ASSERT(!mInterfaceRequestor);
-  aOther.mInterfaceRequestor.swap(mInterfaceRequestor);
-
-  MOZ_ASSERT(!mPrincipalInfo);
-  mPrincipalInfo = aOther.mPrincipalInfo.forget();
-
-  mDomain = aOther.mDomain;
-  mOrigin = aOther.mOrigin;
-  mServiceWorkerCacheName = aOther.mServiceWorkerCacheName;
-  mServiceWorkerDescriptor = aOther.mServiceWorkerDescriptor;
-  mServiceWorkerRegistrationDescriptor = aOther.mServiceWorkerRegistrationDescriptor;
-  mLoadFlags = aOther.mLoadFlags;
-  mWindowID = aOther.mWindowID;
-  mReferrerPolicy = aOther.mReferrerPolicy;
-  mFromWindow = aOther.mFromWindow;
-  mEvalAllowed = aOther.mEvalAllowed;
-  mReportCSPViolations = aOther.mReportCSPViolations;
-  mXHRParamsAllowed = aOther.mXHRParamsAllowed;
-  mPrincipalIsSystem = aOther.mPrincipalIsSystem;
-  mStorageAllowed = aOther.mStorageAllowed;
-  mFirstPartyStorageAccessGranted = aOther.mFirstPartyStorageAccessGranted;
-  mServiceWorkersTestingInWindow = aOther.mServiceWorkersTestingInWindow;
-  mOriginAttributes = aOther.mOriginAttributes;
-  mParentController = aOther.mParentController;
-  mSecureContext = aOther.mSecureContext;
-}
+{}
 
 nsresult
 WorkerLoadInfo::SetPrincipalOnMainThread(nsIPrincipal* aPrincipal,
                                          nsILoadGroup* aLoadGroup)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(NS_LoadGroupMatchesPrincipal(aLoadGroup, aPrincipal));
 
@@ -535,10 +471,26 @@ InterfaceRequestor::GetAnyLiveTabChild()
   return nullptr;
 }
 
 NS_IMPL_ADDREF(WorkerLoadInfo::InterfaceRequestor)
 NS_IMPL_RELEASE(WorkerLoadInfo::InterfaceRequestor)
 NS_IMPL_QUERY_INTERFACE(WorkerLoadInfo::InterfaceRequestor,
                         nsIInterfaceRequestor)
 
+WorkerLoadInfo::WorkerLoadInfo()
+{
+  MOZ_COUNT_CTOR(WorkerLoadInfo);
+}
+
+WorkerLoadInfo::WorkerLoadInfo(WorkerLoadInfo&& aOther) noexcept
+  : WorkerLoadInfoData(std::move(aOther))
+{
+  MOZ_COUNT_CTOR(WorkerLoadInfo);
+}
+
+WorkerLoadInfo::~WorkerLoadInfo()
+{
+  MOZ_COUNT_DTOR(WorkerLoadInfo);
+}
+
 } // dom namespace
 } // mozilla namespace
--- a/dom/workers/WorkerLoadInfo.h
+++ b/dom/workers/WorkerLoadInfo.h
@@ -32,17 +32,17 @@ namespace mozilla {
 namespace ipc {
 class PrincipalInfo;
 } // namespace ipc
 
 namespace dom {
 
 class WorkerPrivate;
 
-struct WorkerLoadInfo
+struct WorkerLoadInfoData
 {
   // All of these should be released in WorkerPrivateParent::ForgetMainThreadObjects.
   nsCOMPtr<nsIURI> mBaseURI;
   nsCOMPtr<nsIURI> mResolvedScriptURI;
 
   // This is the principal of the global (parent worker or a window) loading
   // the worker. It can be null if we are executing a ServiceWorker, otherwise,
   // except for data: URL, it must subsumes the worker principal.
@@ -115,20 +115,31 @@ struct WorkerLoadInfo
   OriginAttributes mOriginAttributes;
 
   enum {
     eNotSet,
     eInsecureContext,
     eSecureContext,
   } mSecureContext;
 
+  WorkerLoadInfoData();
+  WorkerLoadInfoData(WorkerLoadInfoData&& aOther) = default;
+
+  WorkerLoadInfoData&
+  operator=(WorkerLoadInfoData&& aOther) = default;
+};
+
+struct WorkerLoadInfo : WorkerLoadInfoData
+{
   WorkerLoadInfo();
+  WorkerLoadInfo(WorkerLoadInfo&& aOther) noexcept;
   ~WorkerLoadInfo();
 
-  void StealFrom(WorkerLoadInfo& aOther);
+  WorkerLoadInfo&
+  operator=(WorkerLoadInfo&& aOther) = default;
 
   nsresult
   SetPrincipalOnMainThread(nsIPrincipal* aPrincipal, nsILoadGroup* aLoadGroup);
 
   nsresult
   GetPrincipalAndLoadGroupFromChannel(nsIChannel* aChannel,
                                       nsIPrincipal** aPrincipalOut,
                                       nsILoadGroup** aLoadGroupOut);
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -1,16 +1,18 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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 "WorkerPrivate.h"
 
+#include <utility>
+
 #include "js/CompilationAndEvaluation.h"
 #include "js/LocaleSensitive.h"
 #include "js/MemoryMetrics.h"
 #include "js/SourceText.h"
 #include "MessageEventRunnable.h"
 #include "mozilla/ScopeExit.h"
 #include "mozilla/StaticPrefs.h"
 #include "mozilla/dom/BlobURLProtocolHandler.h"
@@ -2239,28 +2241,62 @@ WorkerPrivate::WorkerThreadAccessible::W
   , mFrozen(false)
   , mTimerRunning(false)
   , mRunningExpiredTimeouts(false)
   , mPeriodicGCTimerRunning(false)
   , mIdleGCTimerRunning(false)
   , mOnLine(aParent ? aParent->OnLine() : !NS_IsOffline())
 {}
 
+namespace {
+
+bool
+IsNewWorkerSecureContext(const WorkerPrivate* const aParent,
+                         const WorkerType aWorkerType,
+                         const WorkerLoadInfo& aLoadInfo)
+{
+  if (aParent) {
+    return aParent->IsSecureContext();
+  }
+
+  // Our secure context state depends on the kind of worker we have.
+
+  if (aLoadInfo.mPrincipalIsSystem) {
+    return true;
+  }
+
+  if (aWorkerType == WorkerTypeService) {
+    return true;
+  }
+
+  if (aLoadInfo.mSecureContext != WorkerLoadInfo::eNotSet) {
+    return aLoadInfo.mSecureContext == WorkerLoadInfo::eSecureContext;
+  }
+
+  MOZ_ASSERT_UNREACHABLE("non-chrome worker that is not a service worker "
+                         "that has no parent and no associated window");
+
+  return false;
+}
+
+}
+
 WorkerPrivate::WorkerPrivate(WorkerPrivate* aParent,
                              const nsAString& aScriptURL,
                              bool aIsChromeWorker, WorkerType aWorkerType,
                              const nsAString& aWorkerName,
                              const nsACString& aServiceWorkerScope,
                              WorkerLoadInfo& aLoadInfo)
   : mMutex("WorkerPrivate Mutex")
   , mCondVar(mMutex, "WorkerPrivate CondVar")
   , mParent(aParent)
   , mScriptURL(aScriptURL)
   , mWorkerName(aWorkerName)
   , mWorkerType(aWorkerType)
+  , mLoadInfo(std::move(aLoadInfo))
   , mDebugger(nullptr)
   , mJSContext(nullptr)
   , mPRThread(nullptr)
   , mWorkerControlEventTarget(new WorkerEventTarget(this,
                                                     WorkerEventTarget::Behavior::ControlOnly))
   , mWorkerHybridEventTarget(new WorkerEventTarget(this,
                                                    WorkerEventTarget::Behavior::Hybrid))
   , mParentStatus(Pending)
@@ -2273,58 +2309,44 @@ WorkerPrivate::WorkerPrivate(WorkerPriva
   , mParentWindowPaused(false)
   , mPendingEventQueueClearing(false)
   , mCancelAllPendingRunnables(false)
   , mWorkerScriptExecutedSuccessfully(false)
   , mFetchHandlerWasAdded(false)
   , mMainThreadObjectsForgotten(false)
   , mIsChromeWorker(aIsChromeWorker)
   , mParentFrozen(false)
-  , mIsSecureContext(false)
+  , mIsSecureContext(IsNewWorkerSecureContext(mParent, mWorkerType, mLoadInfo))
   , mDebuggerRegistered(false)
   , mIsInAutomation(false)
   , mPerformanceCounter(nullptr)
 {
   MOZ_ASSERT_IF(!IsDedicatedWorker(), NS_IsMainThread());
-  mLoadInfo.StealFrom(aLoadInfo);
 
   if (aParent) {
     aParent->AssertIsOnWorkerThread();
 
     // Note that this copies our parent's secure context state into mJSSettings.
     aParent->CopyJSSettings(mJSSettings);
 
-    // And manually set our mIsSecureContext, though it's not really relevant to
-    // dedicated workers...
-    mIsSecureContext = aParent->IsSecureContext();
     MOZ_ASSERT_IF(mIsChromeWorker, mIsSecureContext);
 
     mIsInAutomation = aParent->IsInAutomation();
 
     MOZ_ASSERT(IsDedicatedWorker());
 
     if (aParent->mParentFrozen) {
       Freeze(nullptr);
     }
   }
   else {
     AssertIsOnMainThread();
 
     RuntimeService::GetDefaultJSSettings(mJSSettings);
 
-    // Our secure context state depends on the kind of worker we have.
-    if (UsesSystemPrincipal() || IsServiceWorker()) {
-      mIsSecureContext = true;
-    } else if (mLoadInfo.mSecureContext != WorkerLoadInfo::eNotSet) {
-      mIsSecureContext = mLoadInfo.mSecureContext == WorkerLoadInfo::eSecureContext;
-    } else {
-      MOZ_ASSERT_UNREACHABLE("non-chrome worker that is not a service worker "
-                             "that has no parent and no associated window");
-    }
-
     if (mIsSecureContext) {
       mJSSettings.chrome.realmOptions
                  .creationOptions().setSecureContext(true);
       mJSSettings.chrome.realmOptions
                  .creationOptions().setClampAndJitterTime(false);
       mJSSettings.content.realmOptions
                  .creationOptions().setSecureContext(true);
       mJSSettings.content.realmOptions
@@ -2757,17 +2779,17 @@ WorkerPrivate::GetLoadInfo(JSContext* aC
 
     rv = loadInfo.SetPrincipalFromChannel(loadInfo.mChannel);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   MOZ_DIAGNOSTIC_ASSERT(loadInfo.mLoadingPrincipal);
   MOZ_DIAGNOSTIC_ASSERT(loadInfo.PrincipalIsValid());
 
-  aLoadInfo->StealFrom(loadInfo);
+  *aLoadInfo = std::move(loadInfo);
   return NS_OK;
 }
 
 // static
 void
 WorkerPrivate::OverrideLoadInfoLoadGroup(WorkerLoadInfo& aLoadInfo,
                                          nsIPrincipal* aPrincipal)
 {
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -1450,24 +1450,22 @@ private:
   bool mCancelAllPendingRunnables;
   bool mWorkerScriptExecutedSuccessfully;
   bool mFetchHandlerWasAdded;
   bool mMainThreadObjectsForgotten;
   bool mIsChromeWorker;
   bool mParentFrozen;
 
   // mIsSecureContext is set once in our constructor; after that it can be read
-  // from various threads.  We could make this const if we were OK with setting
-  // it in the initializer list via calling some function that takes all sorts
-  // of state (loadinfo, worker type, parent).
+  // from various threads.
   //
   // It's a bit unfortunate that we have to have an out-of-band boolean for
   // this, but we need access to this state from the parent thread, and we can't
   // use our global object's secure state there.
-  bool mIsSecureContext;
+  const bool mIsSecureContext;
 
   bool mDebuggerRegistered;
 
   // mIsInAutomation is true when we're running in test automation.
   // We expose some extra testing functions in that case.
   bool mIsInAutomation;
 
   // This pointer will be null if dom.performance.enable_scheduler_timing is