Bug 1364974: Part 4 - Replace CachedScript::mStatus and related logic with original and final process sets. r=erahm

The current logic is fairly hard to follow, and fails to account for changes
in the set of processes a script needs to be executed in when deciding whether
to write a new cache file. These changes simplify that logic considerably,
increase the chances that we'll correctly pre-decode a script that's needed in
a given process during startup.

MozReview-Commit-ID: BvqjKU8FDHW

--HG--
extra : rebase_source : 132af63ee8f8df5b5d704580d40735c12ed74ed9

js/xpconnect/loader/ScriptPreloader.cpp

@@ -530,39 +530,37 @@ ScriptPreloader::PrepareCacheWrite()
         return;
     }
 
-    bool found = Find(IterHash(mScripts), [] (CachedScript* script) {
-        return (script->mStatus == ScriptStatus::Restored ||
-                !script->HasRange() || script->HasArray());
-    });
-
-    if (!found) {
-        mSaveComplete = true;
-        return;
-    }
-
     AutoSafeJSAPI jsapi;
-
+    bool found = false;
     for (auto& script : IterHash(mScripts, Match<ScriptStatus::Saved>())) {
         // Don't write any scripts that are also in the child cache. They'll be
         // loaded from the child cache in that case, so there's no need to write
         // them twice.
         CachedScript* childScript = mChildCache ? mChildCache->mScripts.Get(script->mCachePath) : nullptr;
-        if (childScript) {
-            if (childScript->mStatus == ScriptStatus::Saved) {
-                childScript->UpdateLoadTime(script->mLoadTime);
-                childScript->mProcessTypes += script->mProcessTypes;
-            } else {
-                childScript = nullptr;
-            }
+        if (childScript && !childScript->mProcessTypes.isEmpty()) {
+            childScript->UpdateLoadTime(script->mLoadTime);
+            childScript->mProcessTypes += script->mProcessTypes;
+            script.Remove();
+            continue;
         }
 
-        if (childScript || (!script->mSize && !script->XDREncode(jsapi.cx()))) {
+        if (!(script->mProcessTypes == script->mOriginalProcessTypes)) {
+            // Note: EnumSet doesn't support operator!=, hence the weird form above.
+            found = true;
+        }
+
+        if (!script->mSize && !script->XDREncode(jsapi.cx())) {
             script.Remove();
         } else {
             script->mSize = script->Range().length();
         }
     }
 
+    if (!found) {
+        mSaveComplete = true;
+        return;
+    }
+
     mDataPrepared = true;
 }
 
@@ -693,9 +691,7 @@ ScriptPreloader::NoteScript(const nsCString& url, const nsCString& cachePath,
 
     auto script = mScripts.LookupOrAdd(cachePath, *this, url, cachePath, jsscript);
 
-    if (script->mStatus == ScriptStatus::Restored) {
-        script->mStatus = ScriptStatus::Saved;
-
+    if (!script->mScript) {
         MOZ_ASSERT(jsscript);
         script->mScript = jsscript;
         script->mReadyToExecute = true;
@@ -712,20 +708,14 @@ ScriptPreloader::NoteScript(const nsCString& url, const nsCString& cachePath,
 {
     auto script = mScripts.LookupOrAdd(cachePath, *this, url, cachePath, nullptr);
 
-    if (script->mStatus == ScriptStatus::Restored) {
-        script->mStatus = ScriptStatus::Saved;
+    if (!script->HasRange()) {
+        MOZ_ASSERT(!script->HasArray());
 
-        script->mReadyToExecute = true;
-    } else {
-        if (!script->HasRange()) {
-            MOZ_ASSERT(!script->HasArray());
+        script->mSize = xdrData.Length();
+        script->mXDRData.construct<nsTArray<uint8_t>>(Forward<nsTArray<uint8_t>>(xdrData));
 
-            script->mSize = xdrData.Length();
-            script->mXDRData.construct<nsTArray<uint8_t>>(Forward<nsTArray<uint8_t>>(xdrData));
-
-            auto& data = script->Array();
-            script->mXDRRange.emplace(data.Elements(), data.Length());
-        }
+        auto& data = script->Array();
+        script->mXDRRange.emplace(data.Elements(), data.Length());
     }
 
     if (!script->mSize && !script->mScript) {
@@ -842,9 +832,14 @@ ScriptPreloader::OffThreadDecodeCallback(void* token, void* context)
 
 ScriptPreloader::CachedScript::CachedScript(ScriptPreloader& cache, InputBuffer& buf)
     : mCache(cache)
-    , mStatus(ScriptStatus::Restored)
 {
     Code(buf);
+
+    // Swap the mProcessTypes and mOriginalProcessTypes values, since we want to
+    // start with an empty set of processes loaded into for this session, and
+    // compare against last session's values later.
+    mOriginalProcessTypes = mProcessTypes;
+    mProcessTypes = {};
 }
 
 bool

js/xpconnect/loader/ScriptPreloader.h

@@ -146,7 +146,6 @@ private:
             : mCache(cache)
             , mURL(url)
             , mCachePath(cachePath)
-            , mStatus(ScriptStatus::Saved)
             , mScript(script)
             , mReadyToExecute(true)
         {}
@@ -155,6 +154,11 @@ private:
 
         ~CachedScript() = default;
 
+        ScriptStatus Status() const
+        {
+          return mProcessTypes.isEmpty() ? ScriptStatus::Restored : ScriptStatus::Saved;
+        }
+
         // For use with nsTArray::Sort.
         //
         // Orders scripts by:
@@ -188,7 +192,7 @@ private:
 
             virtual bool Matches(CachedScript* script)
             {
-                return script->mStatus == mStatus;
+                return script->Status() == mStatus;
             }
 
             const ScriptStatus mStatus;
@@ -292,8 +296,6 @@ private:
         // The size of this script's encoded XDR data.
         uint32_t mSize = 0;
 
-        ScriptStatus mStatus;
-
         TimeStamp mLoadTime{};
 
         JS::Heap<JSScript*> mScript;
@@ -311,6 +313,10 @@ private:
         // The set of processes in which this script has been used.
         EnumSet<ProcessType> mProcessTypes{};
 
+        // The set of processes which the script was loaded into during the
+        // last session, as read from the cache file.
+        EnumSet<ProcessType> mOriginalProcessTypes{};
+
         // The read-only XDR data for this script, which was either read from an
         // existing cache file, or generated by encoding a script which was
         // compiled during this session.