Bug 1428745 - Remove support for version parameter from script loader, r=jonco
☠☠ backed out by 38614ffd21d1 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 09 Jan 2018 17:00:49 +0100
changeset 398382 4e53f251c5b83bbb595e6d91ce14243f9381f9ee
parent 398381 dc271c111e3352f23b424f0a041432dac027ee78
child 398383 1d5dc7dfd429af246a1d1aa1854485be1ad4b6ab
push id98736
push useramarchesini@mozilla.com
push dateTue, 09 Jan 2018 16:01:19 +0000
treeherdermozilla-inbound@1d975770bd9a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1428745
milestone59.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 1428745 - Remove support for version parameter from script loader, r=jonco
dom/script/ModuleLoadRequest.cpp
dom/script/ModuleLoadRequest.h
dom/script/ScriptLoadRequest.cpp
dom/script/ScriptLoadRequest.h
dom/script/ScriptLoader.cpp
dom/script/ScriptLoader.h
toolkit/components/telemetry/Histograms.json
--- a/dom/script/ModuleLoadRequest.cpp
+++ b/dom/script/ModuleLoadRequest.cpp
@@ -24,43 +24,40 @@ NS_IMPL_CYCLE_COLLECTION_INHERITED(Modul
                                    mModuleScript,
                                    mImports)
 
 NS_IMPL_ADDREF_INHERITED(ModuleLoadRequest, ScriptLoadRequest)
 NS_IMPL_RELEASE_INHERITED(ModuleLoadRequest, ScriptLoadRequest)
 
 ModuleLoadRequest::ModuleLoadRequest(nsIURI* aURI,
                                      nsIScriptElement* aElement,
-                                     ValidJSVersion aValidJSVersion,
                                      CORSMode aCORSMode,
                                      const SRIMetadata& aIntegrity,
                                      nsIURI* aReferrer,
                                      mozilla::net::ReferrerPolicy aReferrerPolicy,
                                      ScriptLoader* aLoader)
   : ScriptLoadRequest(ScriptKind::eModule,
                       aURI,
                       aElement,
-                      aValidJSVersion,
                       aCORSMode,
                       aIntegrity,
                       aReferrer,
                       aReferrerPolicy),
     mIsTopLevel(true),
     mLoader(aLoader),
     mVisitedSet(new VisitedURLSet())
 {
   mVisitedSet->PutEntry(aURI);
 }
 
 ModuleLoadRequest::ModuleLoadRequest(nsIURI* aURI,
                                      ModuleLoadRequest* aParent)
   : ScriptLoadRequest(ScriptKind::eModule,
                       aURI,
                       aParent->mElement,
-                      aParent->mValidJSVersion,
                       aParent->mCORSMode,
                       SRIMetadata(),
                       aParent->mURI,
                       aParent->mReferrerPolicy),
     mIsTopLevel(false),
     mLoader(aParent->mLoader),
     mVisitedSet(aParent->mVisitedSet)
 {
--- a/dom/script/ModuleLoadRequest.h
+++ b/dom/script/ModuleLoadRequest.h
@@ -40,17 +40,16 @@ class ModuleLoadRequest final : public S
 
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ModuleLoadRequest, ScriptLoadRequest)
 
   // Create a top-level module load request.
   ModuleLoadRequest(nsIURI* aURI,
                     nsIScriptElement* aElement,
-                    ValidJSVersion aValidJSVersion,
                     CORSMode aCORSMode,
                     const SRIMetadata& aIntegrity,
                     nsIURI* aReferrer,
                     mozilla::net::ReferrerPolicy,
                     ScriptLoader* aLoader);
 
   // Create a module load request for an imported module.
   ModuleLoadRequest(nsIURI* aURI,
--- a/dom/script/ScriptLoadRequest.cpp
+++ b/dom/script/ScriptLoadRequest.cpp
@@ -36,17 +36,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(ScriptLoadRequest)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mScript)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 ScriptLoadRequest::ScriptLoadRequest(ScriptKind aKind,
                                      nsIURI* aURI,
                                      nsIScriptElement* aElement,
-                                     ValidJSVersion aValidJSVersion,
                                      mozilla::CORSMode aCORSMode,
                                      const SRIMetadata& aIntegrity,
                                      nsIURI* aReferrer,
                                      mozilla::net::ReferrerPolicy aReferrerPolicy)
   : mKind(aKind)
   , mElement(aElement)
   , mScriptFromHead(false)
   , mProgress(Progress::eLoading)
@@ -60,17 +59,16 @@ ScriptLoadRequest::ScriptLoadRequest(Scr
   , mIsXSLT(false)
   , mIsCanceled(false)
   , mWasCompiledOMT(false)
   , mIsTracking(false)
   , mOffThreadToken(nullptr)
   , mScriptText()
   , mScriptBytecode()
   , mBytecodeOffset(0)
-  , mValidJSVersion(aValidJSVersion)
   , mURI(aURI)
   , mLineNo(1)
   , mCORSMode(aCORSMode)
   , mIntegrity(aIntegrity)
   , mReferrer(aReferrer)
   , mReferrerPolicy(aReferrerPolicy)
 {
 }
--- a/dom/script/ScriptLoadRequest.h
+++ b/dom/script/ScriptLoadRequest.h
@@ -24,21 +24,16 @@ namespace dom {
 class ModuleLoadRequest;
 class ScriptLoadRequestList;
 
 enum class ScriptKind {
   eClassic,
   eModule
 };
 
-enum class ValidJSVersion : bool {
-  eInvalid,
-  eValid
-};
-
 /*
  * A class that handles loading and evaluation of <script> elements.
  */
 
 class ScriptLoadRequest : public nsISupports,
                           private mozilla::LinkedListElement<ScriptLoadRequest>
 {
   typedef LinkedListElement<ScriptLoadRequest> super;
@@ -49,17 +44,16 @@ class ScriptLoadRequest : public nsISupp
 
 protected:
   virtual ~ScriptLoadRequest();
 
 public:
   ScriptLoadRequest(ScriptKind aKind,
                     nsIURI* aURI,
                     nsIScriptElement* aElement,
-                    ValidJSVersion aValidJSVersion,
                     mozilla::CORSMode aCORSMode,
                     const SRIMetadata &aIntegrity,
                     nsIURI* aReferrer,
                     mozilla::net::ReferrerPolicy aReferrerPolicy);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(ScriptLoadRequest)
 
@@ -212,17 +206,16 @@ public:
   // ownership to jsapi.
   mozilla::Vector<char16_t> mScriptText;
 
   // Holds the SRI serialized hash and the script bytecode for non-inline
   // scripts.
   mozilla::Vector<uint8_t> mScriptBytecode;
   uint32_t mBytecodeOffset; // Offset of the bytecode in mScriptBytecode
 
-  ValidJSVersion mValidJSVersion;
   const nsCOMPtr<nsIURI> mURI;
   nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
   nsCOMPtr<nsIPrincipal> mOriginPrincipal;
   nsAutoCString mURL;     // Keep the URI's filename alive during off thread parsing.
   int32_t mLineNo;
   const mozilla::CORSMode mCORSMode;
   const SRIMetadata mIntegrity;
   const nsCOMPtr<nsIURI> mReferrer;
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -1054,22 +1054,21 @@ ScriptLoader::StartLoad(ScriptLoadReques
       contentPolicyType,
       loadGroup,
       prompter,
       nsIRequest::LOAD_NORMAL |
       nsIChannel::LOAD_CLASSIFY_URI);
 
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // To avoid decoding issues, the JSVersion is explicitly guarded here, and the
-  // build-id is part of the JSBytecodeMimeType constant.
+  // To avoid decoding issues, the build-id is part of the JSBytecodeMimeType
+  // constant.
   aRequest->mCacheInfo = nullptr;
   nsCOMPtr<nsICacheInfoChannel> cic(do_QueryInterface(channel));
-  if (cic && nsContentUtils::IsBytecodeCacheEnabled() &&
-      aRequest->mValidJSVersion == ValidJSVersion::eValid) {
+  if (cic && nsContentUtils::IsBytecodeCacheEnabled()) {
     if (!aRequest->IsLoadingSource()) {
       // Inform the HTTP cache that we prefer to have information coming from the
       // bytecode cache instead of the sources, if such entry is already registered.
       LOG(("ScriptLoadRequest (%p): Maybe request bytecode", aRequest));
       cic->PreferAlternativeDataType(nsContentUtils::JSBytecodeMimeType());
     } else {
       // If we are explicitly loading from the sources, such as after a
       // restarted request, we might still want to save the bytecode after.
@@ -1169,73 +1168,16 @@ bool
 ScriptLoader::PreloadURIComparator::Equals(const PreloadInfo& aPi,
                                            nsIURI* const& aURI) const
 {
   bool same;
   return NS_SUCCEEDED(aPi.mRequest->mURI->Equals(aURI, &same)) &&
          same;
 }
 
-/**
- * Returns ValidJSVersion::eValid if aVersionStr is a string of the form '1.n',
- * n = 0, ..., 8, and ValidJSVersion::eInvalid for other strings.
- */
-static ValidJSVersion
-ParseJavascriptVersion(const nsAString& aVersionStr)
-{
-  if (aVersionStr.Length() != 3 || aVersionStr[0] != '1' ||
-      aVersionStr[1] != '.') {
-    return ValidJSVersion::eInvalid;
-  }
-  if ('0' <= aVersionStr[2] && aVersionStr[2] <= '8') {
-    return ValidJSVersion::eValid;
-  }
-  return ValidJSVersion::eInvalid;
-}
-
-static inline bool
-ParseTypeAttribute(const nsAString& aType, ValidJSVersion* aVersion)
-{
-  MOZ_ASSERT(!aType.IsEmpty());
-  MOZ_ASSERT(aVersion);
-  MOZ_ASSERT(*aVersion == ValidJSVersion::eValid);
-
-  nsContentTypeParser parser(aType);
-
-  nsAutoString mimeType;
-  nsresult rv = parser.GetType(mimeType);
-  NS_ENSURE_SUCCESS(rv, false);
-
-  if (!nsContentUtils::IsJavascriptMIMEType(mimeType)) {
-    return false;
-  }
-
-  // Get the version string, and ensure the language supports it.
-  nsAutoString versionName;
-  rv = parser.GetParameter("version", versionName);
-
-  if (rv == NS_ERROR_INVALID_ARG) {
-    Telemetry::Accumulate(Telemetry::SCRIPT_LOADED_WITH_VERSION, false);
-    // Argument not set.
-    return true;
-  }
-
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-
-  *aVersion = ParseJavascriptVersion(versionName);
-  if (*aVersion == ValidJSVersion::eValid) {
-    Telemetry::Accumulate(Telemetry::SCRIPT_LOADED_WITH_VERSION, true);
-    return true;
-  }
-
-  return true;
-}
-
 static bool
 CSPAllowsInlineScript(nsIScriptElement* aElement, nsIDocument* aDocument)
 {
   nsCOMPtr<nsIContentSecurityPolicy> csp;
   // Note: For imports NodePrincipal and the principal of the master are
   // the same.
   nsresult rv = aDocument->NodePrincipal()->GetCsp(getter_AddRefs(csp));
   NS_ENSURE_SUCCESS(rv, false);
@@ -1258,35 +1200,34 @@ CSPAllowsInlineScript(nsIScriptElement* 
                             &allowInlineScript);
   return allowInlineScript;
 }
 
 ScriptLoadRequest*
 ScriptLoader::CreateLoadRequest(ScriptKind aKind,
                                 nsIURI* aURI,
                                 nsIScriptElement* aElement,
-                                ValidJSVersion aValidJSVersion,
                                 CORSMode aCORSMode,
                                 const SRIMetadata& aIntegrity,
                                 mozilla::net::ReferrerPolicy aReferrerPolicy)
 {
   nsIURI* referrer = mDocument->GetDocumentURI();
 
   if (aKind == ScriptKind::eClassic) {
     ScriptLoadRequest* slr = new ScriptLoadRequest(aKind, aURI, aElement,
-                                                   aValidJSVersion, aCORSMode, aIntegrity,
+                                                   aCORSMode, aIntegrity,
                                                    referrer, aReferrerPolicy);
 
     LOG(("ScriptLoader %p creates ScriptLoadRequest %p", this, slr));
     return slr;
   }
 
   MOZ_ASSERT(aKind == ScriptKind::eModule);
-  return new ModuleLoadRequest(aURI, aElement, aValidJSVersion, aCORSMode,
-                               aIntegrity, referrer, aReferrerPolicy, this);
+  return new ModuleLoadRequest(aURI, aElement, aCORSMode, aIntegrity, referrer,
+                               aReferrerPolicy, this);
 }
 
 bool
 ScriptLoader::ProcessScriptElement(nsIScriptElement* aElement)
 {
   // We need a document to evaluate scripts.
   NS_ENSURE_TRUE(mDocument, false);
 
@@ -1305,23 +1246,21 @@ ScriptLoader::ProcessScriptElement(nsISc
   ScriptKind scriptKind =
     aElement->GetScriptIsModule() ? ScriptKind::eModule : ScriptKind::eClassic;
 
   // Step 13. Check that the script is not an eventhandler
   if (IsScriptEventHandler(scriptKind, scriptContent)) {
     return false;
   }
 
-  ValidJSVersion validJSVersion = ValidJSVersion::eValid;
-
   // For classic scripts, check the type attribute to determine language and
   // version. If type exists, it trumps the deprecated 'language='
   if (scriptKind == ScriptKind::eClassic) {
     if (!type.IsEmpty()) {
-      NS_ENSURE_TRUE(ParseTypeAttribute(type, &validJSVersion), false);
+      NS_ENSURE_TRUE(nsContentUtils::IsJavascriptMIMEType(type), false);
     } else if (!hasType) {
       // no 'type=' element
       // "language" is a deprecated attribute of HTML, so we check it only for
       // HTML script elements.
       if (scriptContent->IsHTMLElement()) {
         nsAutoString language;
         scriptContent->AsElement()->GetAttr(kNameSpaceID_None,
                                             nsGkAtoms::language,
@@ -1424,19 +1363,18 @@ ScriptLoader::ProcessScriptElement(nsISc
         }
       }
 
       nsCOMPtr<nsIPrincipal> principal = aElement->GetScriptURITriggeringPrincipal();
       if (!principal) {
         principal = scriptContent->NodePrincipal();
       }
 
-      request = CreateLoadRequest(scriptKind, scriptURI, aElement,
-                                  validJSVersion, ourCORSMode, sriMetadata,
-                                  ourRefPolicy);
+      request = CreateLoadRequest(scriptKind, scriptURI, aElement, ourCORSMode,
+                                  sriMetadata, ourRefPolicy);
       request->mTriggeringPrincipal = Move(principal);
       request->mIsInline = false;
       request->SetScriptMode(aElement->GetScriptDeferred(),
                              aElement->GetScriptAsync());
       // keep request->mScriptFromHead to false so we don't treat non preloaded
       // scripts as blockers for full page load. See bug 792438.
 
       rv = StartLoad(request);
@@ -1451,18 +1389,16 @@ ScriptLoader::ProcessScriptElement(nsISc
         return false;
       }
     }
 
     // Should still be in loading stage of script.
     NS_ASSERTION(!request->InCompilingStage(),
                  "Request should not yet be in compiling stage.");
 
-    request->mValidJSVersion = validJSVersion;
-
     if (request->IsAsyncScript()) {
       AddAsyncRequest(request);
       if (request->IsReadyToRun()) {
         // The script is available already. Run it ASAP when the event
         // loop gets a chance to spin.
 
         // KVKV TODO: Instead of processing immediately, try off-thread-parsing
         // it and only schedule a pending ProcessRequest if that fails.
@@ -1555,20 +1491,19 @@ ScriptLoader::ProcessScriptElement(nsISc
 
   // Inline classic scripts ignore their CORS mode and are always CORS_NONE.
   CORSMode corsMode = CORS_NONE;
   if (scriptKind == ScriptKind::eModule) {
     corsMode = aElement->GetCORSMode();
   }
 
   request = CreateLoadRequest(scriptKind, mDocument->GetDocumentURI(), aElement,
-                              validJSVersion, corsMode,
+                              corsMode,
                               SRIMetadata(), // SRI doesn't apply
                               ourRefPolicy);
-  request->mValidJSVersion = validJSVersion;
   request->mIsInline = true;
   request->mTriggeringPrincipal = mDocument->NodePrincipal();
   request->mLineNo = aElement->GetScriptLineNumber();
   request->mProgress = ScriptLoadRequest::Progress::eLoading_Source;
   request->mDataType = ScriptLoadRequest::DataType::eSource;
   TRACE_FOR_TEST_BOOL(request->mElement, "scriptloader_load_source");
   CollectScriptTelemetry(nullptr, request);
 
@@ -2187,20 +2122,16 @@ ScriptLoader::EvaluateScript(ScriptLoadR
   // Make sure context is a strong reference since we access it after
   // we've executed a script, which may cause all other references to
   // the context to go away.
   nsCOMPtr<nsIScriptContext> context = globalObject->GetScriptContext();
   if (!context) {
     return NS_ERROR_FAILURE;
   }
 
-  if (aRequest->mValidJSVersion == ValidJSVersion::eInvalid) {
-    return NS_OK;
-  }
-
   // New script entry point required, due to the "Create a script" sub-step of
   // http://www.whatwg.org/specs/web-apps/current-work/#execute-the-script-block
   nsAutoMicroTask mt;
   AutoEntryScript aes(globalObject, "<script> element", true);
   JSContext* cx = aes.cx();
   JS::Rooted<JSObject*> global(cx, globalObject->GetGlobalJSObject());
 
   bool oldProcessingScriptTag = context->GetProcessingScriptTag();
@@ -3205,17 +3136,17 @@ ScriptLoader::PreloadURI(nsIURI* aURI, c
     nsAutoCString sourceUri;
     if (mDocument->GetDocumentURI()) {
       mDocument->GetDocumentURI()->GetAsciiSpec(sourceUri);
     }
     SRICheck::IntegrityMetadata(aIntegrity, sourceUri, mReporter, &sriMetadata);
   }
 
   RefPtr<ScriptLoadRequest> request =
-    CreateLoadRequest(ScriptKind::eClassic, aURI, nullptr, ValidJSVersion::eValid,
+    CreateLoadRequest(ScriptKind::eClassic, aURI, nullptr,
                       Element::StringToCORSMode(aCrossOrigin), sriMetadata,
                       aReferrerPolicy);
   request->mTriggeringPrincipal = mDocument->NodePrincipal();
   request->mIsInline = false;
   request->mScriptFromHead = aScriptFromHead;
   request->SetScriptMode(aDefer, aAsync);
 
   nsresult rv = StartLoad(request);
--- a/dom/script/ScriptLoader.h
+++ b/dom/script/ScriptLoader.h
@@ -337,17 +337,16 @@ public:
   }
 
 private:
   virtual ~ScriptLoader();
 
   ScriptLoadRequest* CreateLoadRequest(ScriptKind aKind,
                                        nsIURI* aURI,
                                        nsIScriptElement* aElement,
-                                       ValidJSVersion aValidJSVersion,
                                        mozilla::CORSMode aCORSMode,
                                        const SRIMetadata& aIntegrity,
                                        mozilla::net::ReferrerPolicy aReferrerPolicy);
 
   /**
    * Unblocks the creator parser of the parser-blocking scripts.
    */
   void UnblockParser(ScriptLoadRequest* aParserBlockingRequest);
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -13857,24 +13857,16 @@
     "alert_emails": ["gfx-telemetry-alerts@mozilla.com","danderson@mozilla.com"],
     "expires_in_version": "61",
     "kind": "exponential",
     "high": 200,
     "n_buckets": 50,
     "description": "Amount of time in milliseconds the main thread spends waiting for the paint thread to complete, if the time was greater than 200us.",
     "bug_numbers": [1386968]
   },
-  "SCRIPT_LOADED_WITH_VERSION": {
-    "record_in_processes": ["main", "content"],
-    "alert_emails": ["amarchesini@mozilla.com"],
-    "bug_numbers": [1418860],
-    "expires_in_version": "60",
-    "kind": "boolean",
-    "description": "Tracking how often scripts are loaded with a 'valid' version parameter. This telemetry ID helps us to decide if and when remove the support of script versioning."
-  },
   "STYLO_PARALLEL_RESTYLE_FRACTION": {
     "record_in_processes": ["content"],
     "alert_emails": ["manish@mozilla.com"],
     "expires_in_version": "60",
     "kind": "linear",
     "high": 100,
     "n_buckets": 50,
     "description": "Percentage of restyles on a single page that were parallel",