From 4807d63e4f846767d9b52f91956843dd71436f64 Mon Sep 17 00:00:00 2001 From: John Schoenick Date: Thu, 4 Oct 2012 12:57:24 -0700 Subject: [PATCH] Bug 797043 - Better handling of plugin activated state for click-to-play. r=josh --- .../test/browser_pluginnotification.js | 61 +++++++++++++++++++ .../base/public/nsIObjectLoadingContent.idl | 9 ++- content/base/src/nsObjectLoadingContent.cpp | 52 +++++++++++++--- content/base/src/nsObjectLoadingContent.h | 11 +++- 4 files changed, 119 insertions(+), 14 deletions(-) diff --git a/browser/base/content/test/browser_pluginnotification.js b/browser/base/content/test/browser_pluginnotification.js index fd608a427f9..0d4fe972251 100644 --- a/browser/base/content/test/browser_pluginnotification.js +++ b/browser/base/content/test/browser_pluginnotification.js @@ -911,5 +911,66 @@ function test21e() { 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(); } diff --git a/content/base/public/nsIObjectLoadingContent.idl b/content/base/public/nsIObjectLoadingContent.idl index 600662ced1e..1a343c53f48 100644 --- a/content/base/public/nsIObjectLoadingContent.idl +++ b/content/base/public/nsIObjectLoadingContent.idl @@ -49,6 +49,8 @@ interface nsIObjectLoadingContent : nsISupports 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) @@ -110,13 +112,14 @@ interface nsIObjectLoadingContent : nsISupports /** * 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; diff --git a/content/base/src/nsObjectLoadingContent.cpp b/content/base/src/nsObjectLoadingContent.cpp index a66ee7e0153..293355977ab 100644 --- a/content/base/src/nsObjectLoadingContent.cpp +++ b/content/base/src/nsObjectLoadingContent.cpp @@ -1470,9 +1470,15 @@ nsObjectLoadingContent::UpdateObjectParameters() 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; } @@ -1553,6 +1559,12 @@ nsObjectLoadingContent::LoadObject(bool aNotify, } } + // 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 @@ -1661,6 +1673,13 @@ nsObjectLoadingContent::LoadObject(bool aNotify, 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 || @@ -2464,15 +2483,25 @@ 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; } @@ -2490,11 +2519,14 @@ nsObjectLoadingContent::CancelPlayPreview() if (!nsContentUtils::IsCallerChrome()) return NS_ERROR_NOT_AVAILABLE; - if (mPlayPreviewCanceled || mActivated) - return NS_OK; - mPlayPreviewCanceled = true; - return LoadObject(true, true); + + // If we're in play preview state already, reload + if (mType == eType_Null && mFallbackType == eFallbackPlayPreview) { + return LoadObject(true, true); + } + + return NS_OK; } bool diff --git a/content/base/src/nsObjectLoadingContent.h b/content/base/src/nsObjectLoadingContent.h index d5ee66b090d..d73bd5b23e9 100644 --- a/content/base/src/nsObjectLoadingContent.h +++ b/content/base/src/nsObjectLoadingContent.h @@ -77,6 +77,8 @@ class nsObjectLoadingContent : public nsImageLoadingContent 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) @@ -222,7 +224,14 @@ class nsObjectLoadingContent : public nsImageLoadingContent 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) }; /**