Bug 767633 - Part 1 - Cleanup nsObjectLoadingContent logic a bit. r=josh
authorJohn Schoenick <jschoenick@mozilla.com>
Thu, 06 Dec 2012 15:09:10 -0800
changeset 115980 6aed5f5caecb8eb5fe6fb610d70bf08452e52587
parent 115979 cc19d255d3510b2b759d5a92afeeb1451125249a
child 115981 1b68ff48e22c35a471bfed05e8222929399a9b53
push id24034
push useremorley@mozilla.com
push dateFri, 14 Dec 2012 15:28:57 +0000
treeherdermozilla-central@50d8f411d305 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjosh
bugs767633
milestone20.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 767633 - Part 1 - Cleanup nsObjectLoadingContent logic a bit. r=josh
browser/base/content/test/browser_pluginnotification.js
content/base/src/nsObjectLoadingContent.cpp
content/base/src/nsObjectLoadingContent.h
--- a/browser/base/content/test/browser_pluginnotification.js
+++ b/browser/base/content/test/browser_pluginnotification.js
@@ -987,20 +987,22 @@ function test23() {
 
   // Activate
   objLoadingContent.playPlugin();
   is(objLoadingContent.displayedType, Ci.nsIObjectLoadingContent.TYPE_PLUGIN, "Test 23, plugin should have started");
   ok(pluginNode.activated, "Test 23, plugin should be activated");
 
   // Reload plugin (this may need RunSoon() in the future when plugins change state asynchronously)
   pluginNode.type = null;
-  pluginNode.src = pluginNode.src; // We currently don't properly change state just on type change, bug 767631
+  // We currently don't properly change state just on type change,
+  // so rebind the plugin to tree. bug 767631
+  pluginNode.parentNode.appendChild(pluginNode);
   is(objLoadingContent.displayedType, Ci.nsIObjectLoadingContent.TYPE_NULL, "Test 23, plugin should be unloaded");
   pluginNode.type = "application/x-test";
-  pluginNode.src = pluginNode.src;
+  pluginNode.parentNode.appendChild(pluginNode);
   is(objLoadingContent.displayedType, Ci.nsIObjectLoadingContent.TYPE_NULL, "Test 23, Plugin should not have activated");
   is(objLoadingContent.pluginFallbackType, Ci.nsIObjectLoadingContent.PLUGIN_CLICK_TO_PLAY, "Test 23, Plugin should be click-to-play");
   ok(!pluginNode.activated, "Test 23, plugin node should not be activated");
 
   prepareTest(test24a, gHttpTestRoot + "plugin_test.html");
 }
 
 // Test that "always allow"-ing a plugin will not allow it when it becomes
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -678,22 +678,22 @@ nsObjectLoadingContent::InstantiatePlugi
   // Instantiating an instance can result in script execution, which
   // can destroy this DOM object. Don't allow that for the scope
   // of this method.
   nsCOMPtr<nsIObjectLoadingContent> kungFuDeathGrip = this;
 
   // Flush layout so that the frame is created if possible and the plugin is
   // initialized with the latest information.
   doc->FlushPendingNotifications(Flush_Layout);
-  
+
   if (!thisContent->GetPrimaryFrame()) {
     LOG(("OBJLC [%p]: Not instantiating plugin with no frame", this));
     return NS_OK;
   }
-  
+
   nsresult rv = NS_ERROR_FAILURE;
   nsRefPtr<nsPluginHost> pluginHost =
     already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
 
   if (!pluginHost) {
     NS_NOTREACHED("No pluginhost");
     return NS_ERROR_FAILURE;
   }
@@ -1339,100 +1339,110 @@ nsObjectLoadingContent::UpdateObjectPara
 
     // Channel can change our URI through redirection
     rv = NS_GetFinalChannelURI(mChannel, getter_AddRefs(newURI));
     if (NS_FAILED(rv)) {
       NS_NOTREACHED("NS_GetFinalChannelURI failure");
       stateInvalid = true;
     }
 
-    // The channel type overrides the guessed / provided type, except when:
+    ObjectType typeHint = newMime.IsEmpty() ?
+                          eType_Null : GetTypeOfContent(newMime);
+
+    //
+    // In order of preference:
     //
-    // 1) If the channel returns a binary stream type, and we have a type hint
-    //    for a non-document (we never want to display binary-as-document),
-    //    use our type hint instead.
-    // 2) Our type hint is a type that we support with a plugin, ignore the
-    //    server's type
+    // 1) Use our type hint if it matches a plugin
+    // 2) If we have eAllowPluginSkipChannel, use the uri file extension if
+    //    it matches a plugin
+    // 3) If the channel returns a binary stream type:
+    //    3a) If we have a type non-null non-document type hint, use that
+    //    3b) If the uri file extension matches a plugin type, use that
+    // 4) Use the channel type
     //
     //    XXX(johns): HTML5's "typesmustmatch" attribute would need to be
     //                honored here if implemented
 
-    ObjectType typeHint = newMime.IsEmpty() ? eType_Null : GetTypeOfContent(newMime);
+    bool overrideChannelType = false;
+    if (typeHint == eType_Plugin) {
+      LOG(("OBJLC [%p]: Using plugin type hint in favor of any channel type",
+           this));
+      overrideChannelType = true;
+    } else if ((caps & eAllowPluginSkipChannel) &&
+               IsPluginEnabledByExtension(newURI, newMime)) {
+      LOG(("OBJLC [%p]: Using extension as type hint for "
+           "eAllowPluginSkipChannel tag (%s)", this, newMime.get()));
+      overrideChannelType = true;
+    } else if (binaryChannelType &&
+               typeHint != eType_Null && typeHint != eType_Document) {
+      LOG(("OBJLC [%p]: Using type hint in favor of binary channel type",
+           this));
+      overrideChannelType = true;
+    } else if (binaryChannelType &&
+               IsPluginEnabledByExtension(newURI, newMime)) {
+      LOG(("OBJLC [%p]: Using extension as type hint for binary channel (%s)",
+           this, newMime.get()));
+      overrideChannelType = true;
+    }
 
-    bool caseOne = binaryChannelType
-                   && typeHint != eType_Null
-                   && typeHint != eType_Document;
-    bool caseTwo = typeHint == eType_Plugin;
-    if (caseOne || caseTwo) {
-        // Set the type we'll use for dispatch on the channel.  Otherwise we could
-        // end up trying to dispatch to a nsFrameLoader, which will complain that
-        // it couldn't find a way to handle application/octet-stream
-        nsAutoCString typeHint, dummy;
-        NS_ParseContentType(newMime, typeHint, dummy);
-        if (!typeHint.IsEmpty()) {
-          mChannel->SetContentType(typeHint);
-        }
-    } else if (binaryChannelType
-               && IsPluginEnabledByExtension(newURI, newMime)) {
-      mChannel->SetContentType(newMime);
+    if (overrideChannelType) {
+      // Set the type we'll use for dispatch on the channel.  Otherwise we could
+      // end up trying to dispatch to a nsFrameLoader, which will complain that
+      // it couldn't find a way to handle application/octet-stream
+      nsAutoCString parsedMime, dummy;
+      NS_ParseContentType(newMime, parsedMime, dummy);
+      if (!parsedMime.IsEmpty()) {
+        mChannel->SetContentType(parsedMime);
+      }
     } else {
       newMime = channelType;
       if (nsPluginHost::IsJavaMIMEType(newMime.get())) {
-        //   Java does not load with a channel, and being java retroactively changes
-        //   how we may have interpreted the codebase to construct this URI above.
-        //   Because the behavior here is more or less undefined, play it safe and
-        //   reject the load.
+        // Java does not load with a channel, and being java retroactively
+        // changes how we may have interpreted the codebase to construct this
+        // URI above.  Because the behavior here is more or less undefined, play
+        // it safe and reject the load.
         LOG(("OBJLC [%p]: Refusing to load with channel with java MIME",
              this));
         stateInvalid = true;
       }
     }
   }
 
   if (useChannel && !mChannel) {
     // - (useChannel && !mChannel) is true if a channel was opened but
     //   is no longer around, in which case we can't load.
     stateInvalid = true;
   }
 
   ///
   /// Determine final type
   ///
+  // In order of preference:
   //  1) If we have attempted channel load, or set stateInvalid above, the type
   //     is always null (fallback)
-  //  2) Otherwise, If we have a loaded channel, we grabbed its mimeType above,
-  //     use that type.
-  //  3) Otherwise, See if we can load this as a plugin without a channel
-  //     (image/document types always need a channel).
-  //     - If we have indication this is a plugin (mime, extension)
-  //       AND:
-  //       - We have eAllowPluginSkipChannel OR
-  //       - We have no URI in the first place (including java)
-  //  3) Otherwise, if we have a URI, set type to loading to indicate
-  //     we'd need a channel to proceed.
-  //  4) Otherwise, type null to indicate unloadable content (fallback)
-  //
-  // XXX(johns): <embed> tags both support URIs and have
-  //   eAllowPluginSkipChannel, meaning it is possible that we have a URI, but
-  //   are not going to open a channel for it. The old objLC code did this (in a
-  //   less obviously-intended way), so it's probably best not to change our
-  //   behavior at this point.
+  //  2) If we have a loaded channel, we grabbed its mimeType above, use that
+  //     type.
+  //  3) If we have a plugin type and no URI, use that type.
+  //  4) If we have a plugin type and eAllowPluginSkipChannel, use that type.
+  //  5) if we have a URI, set type to loading to indicate we'd need a channel
+  //     to proceed.
+  //  6) Otherwise, type null to indicate unloadable content (fallback)
   //
 
   if (stateInvalid) {
     newType = eType_Null;
     newMime.Truncate();
   } else if (useChannel) {
       // If useChannel is set above, we considered it in setting newMime
       newType = GetTypeOfContent(newMime);
       LOG(("OBJLC [%p]: Using channel type", this));
   } else if (((caps & eAllowPluginSkipChannel) || !newURI) &&
-             (GetTypeOfContent(newMime) == eType_Plugin)) {
+             GetTypeOfContent(newMime) == eType_Plugin) {
     newType = eType_Plugin;
-    LOG(("OBJLC [%p]: Skipping loading channel, type plugin", this));
+    LOG(("OBJLC [%p]: Plugin type with no URI, skipping channel load", this));
   } else if (newURI) {
     // We could potentially load this if we opened a channel on mURI, indicate
     // This by leaving type as loading
     newType = eType_Loading;
   } else {
     // Unloadable - no URI, and no plugin type. Non-plugin types (images,
     // documents) always load with a channel.
     newType = eType_Null;
@@ -1581,17 +1591,17 @@ nsObjectLoadingContent::LoadObject(bool 
   UnloadObject(false); // Don't reset state
   if (!mIsLoading) {
     // The event loop must've spun and re-entered into LoadObject, which
     // finished the load
     LOG(("OBJLC [%p]: Re-entered into LoadObject, aborting outer load", this));
     return NS_OK;
   }
 
-  // Determine what's going on with our channel
+  // Determine what's going on with our channel.
   if (stateChange & eParamChannelChanged) {
     // If the channel params changed, throw away the channel, but unset
     // mChannelLoaded so we'll still try to open a new one for this load if
     // necessary
     CloseChannel();
     mChannelLoaded = false;
   } else if (mType == eType_Null && mChannel) {
     // If we opened a channel but then failed to find a loadable state, throw it
@@ -1634,17 +1644,17 @@ nsObjectLoadingContent::LoadObject(bool 
     }
 
     // Content policy implementations can mutate the DOM, check for re-entry
     if (!mIsLoading) {
       LOG(("OBJLC [%p]: We re-entered in content policy, leaving original load",
            this));
       return NS_OK;
     }
-    
+
     // Load denied, switch to fallback and set disabled/suppressed if applicable
     if (!allowLoad) {
       LOG(("OBJLC [%p]: Load denied by policy", this));
       mType = eType_Null;
       if (contentPolicy == nsIContentPolicy::REJECT_TYPE) {
         // XXX(johns) This is assuming that we were rejected by
         //            nsContentBlocker, which rejects by type if permissions
         //            reject plugins
@@ -1735,17 +1745,17 @@ nsObjectLoadingContent::LoadObject(bool 
 
         if (!thisContent->GetPrimaryFrame()) {
           // We're un-rendered, and can't instantiate a plugin. HasNewFrame will
           // re-start us when we can proceed.
           LOG(("OBJLC [%p]: Aborting load - plugin-type, but no frame", this));
           CloseChannel();
           break;
         }
-        
+
         rv = pluginHost->NewEmbeddedPluginStreamListener(mURI, this, nullptr,
                                                          getter_AddRefs(finalListener));
         // finalListener will receive OnStartRequest below
       } else {
         rv = AsyncStartPluginInstance();
       }
     }
     break;
@@ -1753,25 +1763,25 @@ nsObjectLoadingContent::LoadObject(bool 
     {
       if (!mChannel) {
         // We could mFrameLoader->LoadURI(mURI), but UpdateObjectParameters
         // requires documents have a channel, so this is not a valid state.
         NS_NOTREACHED("Attempting to load a document without a channel");
         mType = eType_Null;
         break;
       }
-      
+
       mFrameLoader = nsFrameLoader::Create(thisContent->AsElement(),
                                            mNetworkCreated);
       if (!mFrameLoader) {
         NS_NOTREACHED("nsFrameLoader::Create failed");
         mType = eType_Null;
         break;
       }
-      
+
       rv = mFrameLoader->CheckForRecursiveLoad(mURI);
       if (NS_FAILED(rv)) {
         LOG(("OBJLC [%p]: Aborting recursive load", this));
         mFrameLoader->Destroy();
         mFrameLoader = nullptr;
         mType = eType_Null;
         break;
       }
@@ -1845,17 +1855,20 @@ nsObjectLoadingContent::LoadObject(bool 
     // (so really this is just setting mFallbackType)
     LoadFallback(fallbackType, false);
   }
 
   // Notify of our final state
   NotifyStateChanged(oldType, oldState, false, aNotify);
 
   //
-  // Pass load on to finalListener if loading with a channel
+  // Pass load on to finalListener if loading with a channel.
+  //
+  // We do this after finishing our notification such that re-entrance doesn't
+  // skip notifications or fire them out of order.
   //
 
   if (!mIsLoading) {
     LOG(("OBJLC [%p]: Re-entered before dispatching to final listener", this));
   } else if (finalListener) {
     NS_ASSERTION(mType != eType_Null && mType != eType_Loading,
                  "We should not have a final listener with a non-loaded type");
     // Note that we always enter into LoadObject() from ::OnStartRequest when
--- a/content/base/src/nsObjectLoadingContent.h
+++ b/content/base/src/nsObjectLoadingContent.h
@@ -213,16 +213,17 @@ class nsObjectLoadingContent : public ns
 
     nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                         nsIContent* aBindingParent,
                         bool aCompileEventHandler);
     void UnbindFromTree(bool aDeep = true,
                         bool aNullParent = true);
 
   private:
+
     // Object parameter changes returned by UpdateObjectParameters
     enum ParameterUpdateFlags {
       eParamNoChange           = 0,
       // Parameters that potentially affect the channel changed
       // - mOriginalURI, mOriginalContentType
       eParamChannelChanged     = 1u << 0,
       // Parameters that affect displayed content changed
       // - mURI, mContentType, mType, mBaseURI