Bug 1416066 - Make ScriptPreloader wait for content-document-loaded to fire before writing cache for privileged processes. draft
authorimjching <jlim@mozilla.com>
Fri, 22 Jun 2018 17:29:08 -0400
changeset 816195 f47da8c5ba63084eb0fc01a75c253fa217264ab9
parent 816184 4cb4330e391e791d9fa0ac8dd03183277381d06c
child 816196 8e9355a75e5d68a0d9ab0f49de0859b5f57e2692
push id115771
push userbmo:jlim@mozilla.com
push dateTue, 10 Jul 2018 17:44:26 +0000
bugs1416066
milestone63.0a1
Bug 1416066 - Make ScriptPreloader wait for content-document-loaded to fire before writing cache for privileged processes. MozReview-Commit-ID: 2ElKqWN0clm
js/xpconnect/loader/ScriptPreloader.cpp
js/xpconnect/loader/ScriptPreloader.h
--- a/js/xpconnect/loader/ScriptPreloader.cpp
+++ b/js/xpconnect/loader/ScriptPreloader.cpp
@@ -29,16 +29,17 @@
 #include "nsNetUtil.h"
 #include "nsProxyRelease.h"
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 #include "xpcpublic.h"
 
 #define STARTUP_COMPLETE_TOPIC "browser-delayed-startup-finished"
 #define DOC_ELEM_INSERTED_TOPIC "document-element-inserted"
+#define CONTENT_DOCUMENT_LOADED_TOPIC "content-document-loaded"
 #define CACHE_WRITE_TOPIC "browser-idle-startup-tasks-finished"
 #define CLEANUP_TOPIC "xpcom-shutdown"
 #define SHUTDOWN_TOPIC "quit-application-granted"
 #define CACHE_INVALIDATE_TOPIC "startupcache-invalidate"
 
 // The maximum time we'll wait for a child process to finish starting up before
 // we send its script data back to the parent.
 constexpr uint32_t CHILD_STARTUP_TIMEOUT_MS = 8000;
@@ -365,17 +366,17 @@ ScriptPreloader::Observe(nsISupports* su
 
         MOZ_ASSERT(mStartupFinished);
         MOZ_ASSERT(XRE_IsParentProcess());
 
         if (mChildCache) {
             Unused << NS_NewNamedThread("SaveScripts",
                                         getter_AddRefs(mSaveThread), this);
         }
-    } else if (!strcmp(topic, DOC_ELEM_INSERTED_TOPIC)) {
+    } else if (mContentStartupFinishedTopic.Equals(topic)) {
         // If this is an uninitialized about:blank viewer or a chrome: document
         // (which should always be an XBL binding document), ignore it. We don't
         // have to worry about it loading malicious content.
         if (nsCOMPtr<nsIDocument> doc = do_QueryInterface(subject)) {
             nsCOMPtr<nsIURI> uri = doc->GetDocumentURI();
 
             bool schemeIs;
             if ((NS_IsAboutBlank(uri) &&
@@ -398,18 +399,26 @@ ScriptPreloader::Observe(nsISupports* su
     return NS_OK;
 }
 
 void
 ScriptPreloader::FinishContentStartup()
 {
     MOZ_ASSERT(XRE_IsContentProcess());
 
+#ifdef DEBUG
+    if (mContentStartupFinishedTopic.Equals(CONTENT_DOCUMENT_LOADED_TOPIC)) {
+        MOZ_ASSERT(sProcessType == ProcessType::Privileged);
+    } else {
+        MOZ_ASSERT(sProcessType != ProcessType::Privileged);
+    }
+#endif /* DEBUG */
+
     nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
-    obs->RemoveObserver(this, DOC_ELEM_INSERTED_TOPIC);
+    obs->RemoveObserver(this, mContentStartupFinishedTopic.get());
 
     mSaveTimer = nullptr;
 
     mStartupFinished = true;
 
     if (mChildActor) {
         mChildActor->SendScriptsAndFinalize(mScripts);
     }
@@ -499,21 +508,30 @@ ScriptPreloader::InitCache(const Maybe<i
 
     mCacheInitialized = true;
     mChildActor = cacheChild;
     sProcessType = GetChildProcessType(dom::ContentChild::GetSingleton()->GetRemoteType());
 
     nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
     MOZ_RELEASE_ASSERT(obs);
 
-    // In the child process, we need to freeze the script cache before any
-    // untrusted code has been executed. The insertion of the first DOM
-    // document element may sometimes be earlier than is ideal, but at
-    // least it should always be safe.
-    obs->AddObserver(this, DOC_ELEM_INSERTED_TOPIC, false);
+    if (sProcessType == ProcessType::Privileged) {
+        // Since we control all of the documents loaded in the privileged
+        // content process, we can increase the window of active time for the
+        // ScriptPreloader to include the scripts that are loaded until the
+        // first document finishes loading.
+        mContentStartupFinishedTopic.AssignLiteral(CONTENT_DOCUMENT_LOADED_TOPIC);
+    } else {
+        // In the child process, we need to freeze the script cache before any
+        // untrusted code has been executed. The insertion of the first DOM
+        // document element may sometimes be earlier than is ideal, but at
+        // least it should always be safe.
+        mContentStartupFinishedTopic.AssignLiteral(DOC_ELEM_INSERTED_TOPIC);
+    }
+    obs->AddObserver(this, mContentStartupFinishedTopic.get(), false);
 
     RegisterWeakMemoryReporter(this);
 
     auto cleanup = MakeScopeExit([&] {
         // If the parent is expecting cache data from us, make sure we send it
         // before it writes out its cache file. For normal proceses, this isn't
         // a concern, since they begin loading documents quite early. For the
         // preloaded process, we may end up waiting a long time (or, indeed,
--- a/js/xpconnect/loader/ScriptPreloader.h
+++ b/js/xpconnect/loader/ScriptPreloader.h
@@ -487,16 +487,17 @@ private:
     // The process types for which remote processes have been initialized, and
     // are expected to send back script data.
     EnumSet<ProcessType> mInitializedProcesses{};
 
     RefPtr<ScriptPreloader> mChildCache;
     ScriptCacheChild* mChildActor = nullptr;
 
     nsString mBaseName;
+    nsCString mContentStartupFinishedTopic;
 
     nsCOMPtr<nsIFile> mProfD;
     nsCOMPtr<nsIThread> mSaveThread;
     nsCOMPtr<nsITimer> mSaveTimer;
 
     // The mmapped cache data from this session's cache file.
     AutoMemMap mCacheData;