Bug 797043 - Better handling of plugin activated state for click-to-play. r=josh
authorJohn Schoenick <jschoenick@mozilla.com>
Thu, 04 Oct 2012 12:57:24 -0700
changeset 110595 47a7448b83c6cd5a262197fa02de8c8f7caaef9e
parent 110594 8c10557ef1b5078ce3af6d64b6157036a686d6ef
child 110596 5d9cfce3984b2e9ad56a00c339c5e600965e4c18
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersjosh
bugs797043
milestone19.0a1
Bug 797043 - Better handling of plugin activated state for click-to-play. r=josh
browser/base/content/test/browser_pluginnotification.js
content/base/public/nsIObjectLoadingContent.idl
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
@@ -906,10 +906,71 @@ function test21e() {
     var plugin = doc.getElementById(id);
     var rect = doc.getAnonymousElementByAttribute(plugin, "class", "mainBox").getBoundingClientRect();
     ok(rect.width == 0, "Test 21e, Plugin with id=" + plugin.id + " overlay rect should have 0px width after being clicked");
     ok(rect.height == 0, "Test 21e, Plugin with id=" + plugin.id + " overlay rect should have 0px height after being clicked");
     var objLoadingContent = plugin.QueryInterface(Ci.nsIObjectLoadingContent);
     ok(objLoadingContent.activated, "Test 21e, Plugin with id=" + plugin.id + " should be activated");
   }
 
+  Services.prefs.setBoolPref("plugins.click_to_play", true);
+  prepareTest(test22, gTestRoot + "plugin_test.html");
+}
+
+// Tests that a click-to-play plugin retains its activated state upon reloading
+function test22() {
+  ok(PopupNotifications.getNotification("click-to-play-plugins", gTestBrowser), "Test 22, Should have a click-to-play notification");
+
+  // Plugin should start as CTP
+  var pluginNode = gTestBrowser.contentDocument.getElementById("test");
+  ok(pluginNode, "Test 22, Found plugin in page");
+  var objLoadingContent = pluginNode.QueryInterface(Ci.nsIObjectLoadingContent);
+  is(objLoadingContent.pluginFallbackType, Ci.nsIObjectLoadingContent.PLUGIN_CLICK_TO_PLAY, "Test 22, plugin fallback type should be PLUGIN_CLICK_TO_PLAY");
+
+  // Activate
+  objLoadingContent.playPlugin();
+  is(objLoadingContent.displayedType, Ci.nsIObjectLoadingContent.TYPE_PLUGIN, "Test 22, plugin should have started");
+  ok(pluginNode.activated, "Test 22, plugin should be activated");
+
+  // Reload plugin
+  var oldVal = pluginNode.getObjectValue();
+  pluginNode.src = pluginNode.src;
+  is(objLoadingContent.displayedType, Ci.nsIObjectLoadingContent.TYPE_PLUGIN, "Test 22, Plugin should have retained activated state");
+  ok(pluginNode.activated, "Test 22, plugin should have remained activated");
+  // Sanity, ensure that we actually reloaded the instance, since this behavior might change in the future.
+  var pluginsDiffer;
+  try {
+    pluginNode.checkObjectValue(oldVal);
+  } catch (e) {
+    pluginsDiffer = true;
+  }
+  ok(pluginsDiffer, "Test 22, plugin should have reloaded");
+
+  prepareTest(test23, gTestRoot + "plugin_test.html");
+}
+
+// Tests that a click-to-play plugin resets its activated state when changing types
+function test23() {
+  ok(PopupNotifications.getNotification("click-to-play-plugins", gTestBrowser), "Test 23, Should have a click-to-play notification");
+
+  // Plugin should start as CTP
+  var pluginNode = gTestBrowser.contentDocument.getElementById("test");
+  ok(pluginNode, "Test 23, Found plugin in page");
+  var objLoadingContent = pluginNode.QueryInterface(Ci.nsIObjectLoadingContent);
+  is(objLoadingContent.pluginFallbackType, Ci.nsIObjectLoadingContent.PLUGIN_CLICK_TO_PLAY, "Test 23, plugin fallback type should be PLUGIN_CLICK_TO_PLAY");
+
+  // 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
+  is(objLoadingContent.displayedType, Ci.nsIObjectLoadingContent.TYPE_NULL, "Test 23, plugin should be unloaded");
+  pluginNode.type = "application/x-test";
+  pluginNode.src = pluginNode.src;
+  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");
+
   finishTest();
 }
--- a/content/base/public/nsIObjectLoadingContent.idl
+++ b/content/base/public/nsIObjectLoadingContent.idl
@@ -44,16 +44,18 @@ interface nsIObjectLoadingContent : nsIS
   // The plugin is considered outdated, but not disabled
   const unsigned long PLUGIN_OUTDATED             = 4;
   // The plugin has crashed
   const unsigned long PLUGIN_CRASHED              = 5;
   // Suppressed by security policy
   const unsigned long PLUGIN_SUPPRESSED           = 6;
   // Blocked by content policy
   const unsigned long PLUGIN_USER_DISABLED        = 7;
+  /// ** All values >= PLUGIN_CLICK_TO_PLAY are plugin placeholder types that
+  ///    would be replaced by a real plugin if activated (playPlugin())
   // The plugin is disabled until the user clicks on it
   const unsigned long PLUGIN_CLICK_TO_PLAY        = 8;
   // The plugin is vulnerable (update available)
   const unsigned long PLUGIN_VULNERABLE_UPDATABLE = 9;
   // The plugin is vulnerable (no update available)
   const unsigned long PLUGIN_VULNERABLE_NO_UPDATE = 10;
   // The plugin is in play preview mode
   const unsigned long PLUGIN_PLAY_PREVIEW         = 11;
@@ -105,23 +107,24 @@ interface nsIObjectLoadingContent : nsIS
 
   [noscript] void pluginCrashed(in nsIPluginTag pluginTag,
                                 in AString pluginDumpID,
                                 in AString browserDumpID,
                                 in boolean submittedCrashReport);
 
   /**
    * This method will play a plugin that has been stopped by the
-   * click-to-play plugins feature.
+   * click-to-play plugins or play-preview features.
    */
   void playPlugin();
 
   /**
-   * This attribute will return true if the plugin has been activated and
-   * false if the plugin is still in the click-to-play or play preview state.
+   * This attribute will return true if the current content type has been
+   * activated, either explicitly or by passing checks that would have it be
+   * click-to-play or play-preview.
    */
   readonly attribute boolean activated;
 
   [noscript] void stopPluginInstance();
 
   [noscript] void syncStartPluginInstance();
   [noscript] void asyncStartPluginInstance();
 
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -1465,19 +1465,25 @@ nsObjectLoadingContent::UpdateObjectPara
   }
 
   if (!URIEquals(newURI, mURI)) {
     retval = (ParameterUpdateFlags)(retval | eParamStateChanged);
     LOG(("OBJLC [%p]: Object effective URI changed", this));
     mURI = newURI;
   }
 
-  if (mContentType != newMime) {
+  // We don't update content type when loading, as the type is not final and we
+  // don't want to superfluously change between mOriginalContentType ->
+  // mContentType when doing |obj.data = obj.data| with a channel and differing
+  // type.
+  if (mType != eType_Loading && mContentType != newMime) {
     retval = (ParameterUpdateFlags)(retval | eParamStateChanged);
-    LOG(("OBJLC [%p]: Object effective mime type changed (%s -> %s)", this, mContentType.get(), newMime.get()));
+    retval = (ParameterUpdateFlags)(retval | eParamContentTypeChanged);
+    LOG(("OBJLC [%p]: Object effective mime type changed (%s -> %s)",
+         this, mContentType.get(), newMime.get()));
     mContentType = newMime;
   }
 
   return retval;
 }
 
 // Only OnStartRequest should be passing the channel parameter
 nsresult
@@ -1548,16 +1554,22 @@ nsObjectLoadingContent::LoadObject(bool 
       fallbackType = eFallbackDisabled;
     } else if (pluginsupport == NS_ERROR_PLUGIN_BLOCKLISTED) {
       fallbackType = eFallbackBlocklisted;
     } else {
       fallbackType = eFallbackUnsupported;
     }
   }
 
+  // Explicit user activation should reset if the object changes content types
+  if (mActivated && (stateChange & eParamContentTypeChanged)) {
+    LOG(("OBJLC [%p]: Content type changed, clearing activation state", this));
+    mActivated = false;
+  }
+
   // We synchronously start/stop plugin instances below, which may spin the
   // event loop. Re-entering into the load is fine, but at that point the
   // original load call needs to abort when unwinding
   // NOTE this is located *after* the state change check, a subseqent load
   //      with no subsequently changed state will be a no-op.
   if (mIsLoading) {
     LOG(("OBJLC [%p]: Re-entering into LoadObject", this));
   }
@@ -1656,16 +1668,23 @@ nsObjectLoadingContent::LoadObject(bool 
   // reason click-to-play instead
   FallbackType clickToPlayReason;
   if (mType == eType_Plugin && !ShouldPlay(clickToPlayReason)) {
     LOG(("OBJLC [%p]: Marking plugin as click-to-play", this));
     mType = eType_Null;
     fallbackType = clickToPlayReason;
   }
 
+  if (!mActivated && mType != eType_Null) {
+    // Object passed ShouldPlay and !ShouldPreview, so it should be considered
+    // activated until it changes content type
+    LOG(("OBJLC [%p]: Object implicitly activated", this));
+    mActivated = true;
+  }
+
   // Sanity check: We shouldn't have any loaded resources, pending events, or
   // a final listener at this point
   if (mFrameLoader || mPendingInstantiateEvent || mInstanceOwner ||
       mFinalListener)
   {
     NS_NOTREACHED("Trying to load new plugin with existing content");
     rv = NS_ERROR_UNEXPECTED;
     return NS_OK;
@@ -2459,25 +2478,35 @@ nsObjectLoadingContent::NotifyContentObj
 }
 
 NS_IMETHODIMP
 nsObjectLoadingContent::PlayPlugin()
 {
   if (!nsContentUtils::IsCallerChrome())
     return NS_OK;
 
-  mActivated = true;
-  return LoadObject(true, true);
+  if (!mActivated) {
+    mActivated = true;
+    LOG(("OBJLC [%p]: Activated by user", this));
+  }
+
+  // If we're in a click-to-play or play preview state, we need to reload
+  // Fallback types >= eFallbackClickToPlay are plugin-replacement types, see
+  // header
+  if (mType == eType_Null && mFallbackType >= eFallbackClickToPlay) {
+    return LoadObject(true, true);
+  }
+
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsObjectLoadingContent::GetActivated(bool *aActivated)
 {
-  FallbackType reason;
-  *aActivated = ShouldPlay(reason) && !ShouldPreview();
+  *aActivated = mActivated;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsObjectLoadingContent::GetPluginFallbackType(uint32_t* aPluginFallbackType)
 {
   NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome(), NS_ERROR_NOT_AVAILABLE);
   *aPluginFallbackType = mFallbackType;
@@ -2485,21 +2514,24 @@ nsObjectLoadingContent::GetPluginFallbac
 }
 
 NS_IMETHODIMP
 nsObjectLoadingContent::CancelPlayPreview()
 {
   if (!nsContentUtils::IsCallerChrome())
     return NS_ERROR_NOT_AVAILABLE;
 
-  if (mPlayPreviewCanceled || mActivated)
-    return NS_OK;
+  mPlayPreviewCanceled = true;
+  
+  // If we're in play preview state already, reload
+  if (mType == eType_Null && mFallbackType == eFallbackPlayPreview) {
+    return LoadObject(true, true);
+  }
 
-  mPlayPreviewCanceled = true;
-  return LoadObject(true, true);
+  return NS_OK;
 }
 
 bool
 nsObjectLoadingContent::ShouldPreview()
 {
   if (mPlayPreviewCanceled || mActivated)
     return false;
 
--- a/content/base/src/nsObjectLoadingContent.h
+++ b/content/base/src/nsObjectLoadingContent.h
@@ -72,16 +72,18 @@ class nsObjectLoadingContent : public ns
       // The plugin is considered outdated, but not disabled
       eFallbackOutdated = nsIObjectLoadingContent::PLUGIN_OUTDATED,
       // The plugin has crashed
       eFallbackCrashed = nsIObjectLoadingContent::PLUGIN_CRASHED,
       // Suppressed by security policy
       eFallbackSuppressed = nsIObjectLoadingContent::PLUGIN_SUPPRESSED,
       // Blocked by content policy
       eFallbackUserDisabled = nsIObjectLoadingContent::PLUGIN_USER_DISABLED,
+      /// ** All values >= eFallbackClickToPlay are plugin placeholder types
+      ///    that would be replaced by a real plugin if activated (PlayPlugin())
       // The plugin is disabled until the user clicks on it
       eFallbackClickToPlay = nsIObjectLoadingContent::PLUGIN_CLICK_TO_PLAY,
       // The plugin is vulnerable (update available)
       eFallbackVulnerableUpdatable = nsIObjectLoadingContent::PLUGIN_VULNERABLE_UPDATABLE,
       // The plugin is vulnerable (no update available)
       eFallbackVulnerableNoUpdate = nsIObjectLoadingContent::PLUGIN_VULNERABLE_NO_UPDATE,
       // The plugin is disabled and play preview content is displayed until
       // the extension code enables it by sending the MozPlayPlugin event
@@ -217,17 +219,24 @@ class nsObjectLoadingContent : public ns
     // Object parameter changes returned by UpdateObjectParameters
     enum ParameterUpdateFlags {
       eParamNoChange           = 0,
       // Parameters that potentially affect the channel changed
       // - mOriginalURI, mOriginalContentType
       eParamChannelChanged     = PR_BIT(0),
       // Parameters that affect displayed content changed
       // - mURI, mContentType, mType, mBaseURI
-      eParamStateChanged       = PR_BIT(1)
+      eParamStateChanged       = PR_BIT(1),
+      // The effective content type changed, independant of object type. This
+      // can happen when changing from Loading -> Final type, but doesn't
+      // necessarily happen when changing between object types. E.g., if a PDF
+      // handler was installed between the last load of this object and now, we
+      // might change from eType_Document -> eType_Plugin without changing
+      // ContentType
+      eParamContentTypeChanged = PR_BIT(2)
     };
 
     /**
      * Loads fallback content with the specified FallbackType
      *
      * @param aType   FallbackType value for type of fallback we're loading
      * @param aNotify Send notifications and events. If false, caller is
      *                responsible for doing so