From 0b8691d471831191d7f606d1facc86d932d73f1c Mon Sep 17 00:00:00 2001 From: David Keeler Date: Wed, 27 Mar 2013 14:06:43 -0700 Subject: [PATCH] bug 854467 - fix nsIPluginTag enabled state API r=bsmedberg r=unfocused --- .../test/browser_pluginnotification.js | 10 +-- content/base/test/chrome/test_bug391728.html | 6 +- dom/plugins/base/nsIPluginTag.idl | 14 +++- dom/plugins/base/nsPluginHost.cpp | 2 +- dom/plugins/base/nsPluginTags.cpp | 65 +++++++------------ dom/plugins/base/nsPluginTags.h | 2 + .../test_refresh_navigator_plugins.html | 4 +- dom/plugins/test/unit/test_bug854467.js | 38 +++++++++++ dom/plugins/test/unit/xpcshell.ini | 1 + toolkit/mozapps/extensions/PluginProvider.jsm | 8 ++- .../mozapps/extensions/nsBlocklistService.js | 4 +- .../test/xpcshell/test_bug455906.js | 24 ++++--- .../test/xpcshell/test_duplicateplugins.js | 23 +++++-- 13 files changed, 125 insertions(+), 76 deletions(-) create mode 100644 dom/plugins/test/unit/test_bug854467.js diff --git a/browser/base/content/test/browser_pluginnotification.js b/browser/base/content/test/browser_pluginnotification.js index 1c913f270aad..0809a8c09104 100644 --- a/browser/base/content/test/browser_pluginnotification.js +++ b/browser/base/content/test/browser_pluginnotification.js @@ -110,7 +110,7 @@ function test1() { var plugin = getTestPlugin(); ok(plugin, "Should have a test plugin"); - plugin.disabled = false; + plugin.enabledState = Ci.nsIPluginTag.STATE_ENABLED; plugin.blocklisted = false; prepareTest(test2, gTestRoot + "plugin_test.html"); } @@ -124,7 +124,7 @@ function test2() { var plugin = getTestPlugin(); ok(plugin, "Should have a test plugin"); - plugin.disabled = true; + plugin.enabledState = Ci.nsIPluginTag.STATE_DISABLED; prepareTest(test3, gTestRoot + "plugin_test.html"); } @@ -154,7 +154,7 @@ function test4(tab, win) { function prepareTest5() { var plugin = getTestPlugin(); - plugin.disabled = false; + plugin.enabledState = Ci.nsIPluginTag.STATE_ENABLED; plugin.blocklisted = true; prepareTest(test5, gTestRoot + "plugin_test.html"); } @@ -197,7 +197,7 @@ function test7() { ok(gTestBrowser.missingPlugins.has("application/x-test"), "Test 7, Should know about application/x-test"); var plugin = getTestPlugin(); - plugin.disabled = false; + plugin.enabledState = Ci.nsIPluginTag.STATE_ENABLED; plugin.blocklisted = false; Services.prefs.setBoolPref("plugins.click_to_play", true); @@ -523,7 +523,7 @@ function test14() { ok(objLoadingContent.activated, "Test 14, Plugin should be activated"); var plugin = getTestPlugin(); - plugin.disabled = false; + plugin.enabledState = Ci.nsIPluginTag.STATE_ENABLED; plugin.blocklisted = false; Services.prefs.setBoolPref("plugins.click_to_play", true); prepareTest(test15, gTestRoot + "plugin_alternate_content.html"); diff --git a/content/base/test/chrome/test_bug391728.html b/content/base/test/chrome/test_bug391728.html index f257c8c7d19b..7d8ad1ca0574 100644 --- a/content/base/test/chrome/test_bug391728.html +++ b/content/base/test/chrome/test_bug391728.html @@ -71,7 +71,7 @@ function start_test(plugin) { } function finish_test(plugin) { - plugin.disabled = false; + plugin.enabledState = Components.interfaces.nsIPluginTag.STATE_ENABLED; plugin.blocklisted = false; var prefs = Components.classes["@mozilla.org/preferences-service;1"] .getService(Components.interfaces.nsIPrefBranch); @@ -127,7 +127,7 @@ function test_normal(plugin) { is(gDisabled.length, 0, "Should not have been any disabled plugins"); is(gBlocked.length, 0, "Should not have been any blocked plugins"); test_style("solid"); - plugin.disabled = true; + plugin.enabledState = Components.interfaces.nsIPluginTag.STATE_DISABLED; load_frame(test_disabled, "file_bug391728"); } @@ -138,7 +138,7 @@ function test_disabled(plugin) { is(gBlocked.length, 0, "Should not have been any blocked plugins"); test_style("dotted"); ok(plugin.disabled, "Plugin lost its disabled status"); - plugin.disabled = false; + plugin.enabledState = Components.interfaces.nsIPluginTag.STATE_ENABLED; plugin.blocklisted = true; load_frame(test_blocked, "file_bug391728"); } diff --git a/dom/plugins/base/nsIPluginTag.idl b/dom/plugins/base/nsIPluginTag.idl index 8cee3ca61f71..ef75a27f2406 100644 --- a/dom/plugins/base/nsIPluginTag.idl +++ b/dom/plugins/base/nsIPluginTag.idl @@ -6,17 +6,25 @@ #include "nsISupports.idl" interface nsIDOMMimeType; -[scriptable, uuid(b8bf0a06-e395-4f44-af39-a51d3e7ef4b9)] +[scriptable, uuid(87b4fcfc-417b-47f6-9c79-dfeb5e5a4840)] interface nsIPluginTag : nsISupports { + // enabledState is stored as one of the following as an integer in prefs, + // so if new states are added, they must not renumber the existing states. + const unsigned long STATE_DISABLED = 0; + const unsigned long STATE_CLICKTOPLAY = 1; + const unsigned long STATE_ENABLED = 2; + readonly attribute AUTF8String description; readonly attribute AUTF8String filename; readonly attribute AUTF8String fullpath; readonly attribute AUTF8String version; readonly attribute AUTF8String name; - attribute boolean disabled; attribute boolean blocklisted; - attribute boolean clicktoplay; + readonly attribute boolean disabled; + readonly attribute boolean clicktoplay; + attribute unsigned long enabledState; + void getMimeTypes([optional] out unsigned long aCount, [retval, array, size_is(aCount)] out nsIDOMMimeType aResults); }; diff --git a/dom/plugins/base/nsPluginHost.cpp b/dom/plugins/base/nsPluginHost.cpp index bf11c6cf5eb7..69f403646aef 100644 --- a/dom/plugins/base/nsPluginHost.cpp +++ b/dom/plugins/base/nsPluginHost.cpp @@ -2043,7 +2043,7 @@ nsresult nsPluginHost::ScanPluginsDirectory(nsIFile *pluginsDir, pluginTag->SetBlocklisted(true); } if (state == nsIBlocklistService::STATE_SOFTBLOCKED && !seenBefore) { - pluginTag->SetDisabled(true); + pluginTag->SetEnabledState(nsIPluginTag::STATE_DISABLED); } if (state == nsIBlocklistService::STATE_OUTDATED && !seenBefore) { warnOutdated = true; diff --git a/dom/plugins/base/nsPluginTags.cpp b/dom/plugins/base/nsPluginTags.cpp index cf79c8e9d77c..538d029752ca 100644 --- a/dom/plugins/base/nsPluginTags.cpp +++ b/dom/plugins/base/nsPluginTags.cpp @@ -321,26 +321,6 @@ nsPluginTag::IsEnabled() return (state == ePluginState_Enabled) || (state == ePluginState_Clicktoplay); } -void -nsPluginTag::SetEnabled(bool enabled) -{ - if (enabled == IsEnabled()) { - return; - } - - PluginState state = GetPluginState(); - - if (!enabled) { - SetPluginState(ePluginState_Disabled); - } else if (state != ePluginState_Clicktoplay) { - SetPluginState(ePluginState_Enabled); - } - - if (nsRefPtr host = nsPluginHost::GetInst()) { - host->UpdatePluginInfo(this); - } -} - NS_IMETHODIMP nsPluginTag::GetDisabled(bool* aDisabled) { @@ -348,13 +328,6 @@ nsPluginTag::GetDisabled(bool* aDisabled) return NS_OK; } -NS_IMETHODIMP -nsPluginTag::SetDisabled(bool aDisabled) -{ - SetEnabled(!aDisabled); - return NS_OK; -} - bool nsPluginTag::IsBlocklisted() { @@ -403,19 +376,23 @@ nsPluginTag::GetClicktoplay(bool *aClicktoplay) } NS_IMETHODIMP -nsPluginTag::SetClicktoplay(bool clicktoplay) -{ - if (clicktoplay == IsClicktoplay()) { - return NS_OK; - } +nsPluginTag::GetEnabledState(uint32_t *aEnabledState) { + *aEnabledState = Preferences::GetInt(GetStatePrefNameForPlugin(this).get(), + ePluginState_Enabled); + return NS_OK; +} - const PluginState state = GetPluginState(); - if (state != ePluginState_Disabled) { - SetPluginState(ePluginState_Clicktoplay); - } - - if (nsRefPtr host = nsPluginHost::GetInst()) { - host->UpdatePluginInfo(this); +NS_IMETHODIMP +nsPluginTag::SetEnabledState(uint32_t aEnabledState) { + if (aEnabledState >= ePluginState_MaxValue) + return NS_ERROR_ILLEGAL_VALUE; + uint32_t oldState = nsIPluginTag::STATE_DISABLED; + GetEnabledState(&oldState); + if (oldState != aEnabledState) { + Preferences::SetInt(GetStatePrefNameForPlugin(this).get(), aEnabledState); + if (nsRefPtr host = nsPluginHost::GetInst()) { + host->UpdatePluginInfo(this); + } } return NS_OK; } @@ -423,14 +400,18 @@ nsPluginTag::SetClicktoplay(bool clicktoplay) nsPluginTag::PluginState nsPluginTag::GetPluginState() { - return (PluginState)Preferences::GetInt(GetStatePrefNameForPlugin(this).get(), - ePluginState_Enabled); + uint32_t enabledState = nsIPluginTag::STATE_DISABLED; + GetEnabledState(&enabledState); + return (PluginState)enabledState; } void nsPluginTag::SetPluginState(PluginState state) { - Preferences::SetInt(GetStatePrefNameForPlugin(this).get(), state); + MOZ_STATIC_ASSERT(nsPluginTag::ePluginState_Disabled == nsIPluginTag::STATE_DISABLED, "nsPluginTag::ePluginState_Disabled must match nsIPluginTag::STATE_DISABLED"); + MOZ_STATIC_ASSERT(nsPluginTag::ePluginState_Clicktoplay == nsIPluginTag::STATE_CLICKTOPLAY, "nsPluginTag::ePluginState_Clicktoplay must match nsIPluginTag::STATE_CLICKTOPLAY"); + MOZ_STATIC_ASSERT(nsPluginTag::ePluginState_Enabled == nsIPluginTag::STATE_ENABLED, "nsPluginTag::ePluginState_Enabled must match nsIPluginTag::STATE_ENABLED"); + SetEnabledState((uint32_t)state); } NS_IMETHODIMP diff --git a/dom/plugins/base/nsPluginTags.h b/dom/plugins/base/nsPluginTags.h index 0e7fe5665056..36d6682d6a81 100644 --- a/dom/plugins/base/nsPluginTags.h +++ b/dom/plugins/base/nsPluginTags.h @@ -27,10 +27,12 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSIPLUGINTAG + // These must match the STATE_* values in nsIPluginTag.idl enum PluginState { ePluginState_Disabled = 0, ePluginState_Clicktoplay = 1, ePluginState_Enabled = 2, + ePluginState_MaxValue = 3, }; nsPluginTag(nsPluginTag* aPluginTag); diff --git a/dom/plugins/test/mochitest/test_refresh_navigator_plugins.html b/dom/plugins/test/mochitest/test_refresh_navigator_plugins.html index adb9404d72a3..a8cfc956adae 100644 --- a/dom/plugins/test/mochitest/test_refresh_navigator_plugins.html +++ b/dom/plugins/test/mochitest/test_refresh_navigator_plugins.html @@ -41,7 +41,7 @@ ok(!tagTestPlugin.disabled, "test plugin should not be disabled"); nextTest = testPart2; - tagTestPlugin.disabled = true; + tagTestPlugin.enabledState = Components.interfaces.nsIPluginTag.STATE_DISABLED; function testPart2() { var navTestPlugin = navigator.plugins.namedItem("Test Plug-in"); @@ -49,7 +49,7 @@ ok(!navigator.mimeTypes[mimeType.type], "now navigator.mimeTypes should not have an entry for '" + mimeType.type + "'"); nextTest = testPart3; - tagTestPlugin.disabled = false; + tagTestPlugin.enabledState = Components.interfaces.nsIPluginTag.STATE_ENABLED; } function testPart3() { diff --git a/dom/plugins/test/unit/test_bug854467.js b/dom/plugins/test/unit/test_bug854467.js new file mode 100644 index 000000000000..9aeb56958f49 --- /dev/null +++ b/dom/plugins/test/unit/test_bug854467.js @@ -0,0 +1,38 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +function check_state(aTag, aExpectedClicktoplay, aExpectedDisabled) { + do_check_eq(aTag.clicktoplay, aExpectedClicktoplay); + do_check_eq(aTag.disabled, aExpectedDisabled); +} + +function run_test() { + let tag = get_test_plugintag(); + check_state(tag, false, false); + + /* test going to click-to-play from always enabled and back */ + tag.enabledState = Ci.nsIPluginTag.STATE_CLICKTOPLAY; + check_state(tag, true, false); + tag.enabledState = Ci.nsIPluginTag.STATE_ENABLED; + check_state(tag, false, false); + + /* test going to disabled from always enabled and back */ + tag.enabledState = Ci.nsIPluginTag.STATE_DISABLED; + check_state(tag, false, true); + tag.enabledState = Ci.nsIPluginTag.STATE_ENABLED; + check_state(tag, false, false); + + /* test going to click-to-play from disabled and back */ + tag.enabledState = Ci.nsIPluginTag.STATE_DISABLED; + check_state(tag, false, true); + tag.enabledState = Ci.nsIPluginTag.STATE_CLICKTOPLAY; + check_state(tag, true, false); + tag.enabledState = Ci.nsIPluginTag.STATE_DISABLED; + check_state(tag, false, true); + + /* put everything back to normal and check that that succeeded */ + tag.enabledState = Ci.nsIPluginTag.STATE_ENABLED; + check_state(tag, false, false); +} diff --git a/dom/plugins/test/unit/xpcshell.ini b/dom/plugins/test/unit/xpcshell.ini index 2c779c10a9db..5c50c6e61c9f 100644 --- a/dom/plugins/test/unit/xpcshell.ini +++ b/dom/plugins/test/unit/xpcshell.ini @@ -15,3 +15,4 @@ fail-if = os == "android" # Bug 676953: test fails consistently on Android fail-if = os == "android" [test_persist_in_prefs.js] +[test_bug854467.js] diff --git a/toolkit/mozapps/extensions/PluginProvider.jsm b/toolkit/mozapps/extensions/PluginProvider.jsm index 0127cafa8257..96c7a12e7b6d 100644 --- a/toolkit/mozapps/extensions/PluginProvider.jsm +++ b/toolkit/mozapps/extensions/PluginProvider.jsm @@ -300,8 +300,12 @@ function PluginWrapper(aId, aName, aDescription, aTags) { if (aTags[0].disabled == aVal) return; - for (let tag of aTags) - tag.disabled = aVal; + for (let tag of aTags) { + if (aVal === true) + tag.enabledState = Ci.nsIPluginTag.STATE_DISABLED; + else + tag.enabledState = Ci.nsIPluginTag.STATE_ENABLED; + } AddonManagerPrivate.callAddonListeners(aVal ? "onDisabling" : "onEnabling", this, false); AddonManagerPrivate.callAddonListeners(aVal ? "onDisabled" : "onEnabled", this); return aVal; diff --git a/toolkit/mozapps/extensions/nsBlocklistService.js b/toolkit/mozapps/extensions/nsBlocklistService.js index 94299bbe368a..326be576e3b7 100644 --- a/toolkit/mozapps/extensions/nsBlocklistService.js +++ b/toolkit/mozapps/extensions/nsBlocklistService.js @@ -959,7 +959,7 @@ Blocklist.prototype = { if (plugin.blocklisted) { if (state == Ci.nsIBlocklistService.STATE_SOFTBLOCKED) - plugin.disabled = true; + plugin.enabledState = Ci.nsIPluginTag.STATE_DISABLED; } else if (!plugin.disabled && state != Ci.nsIBlocklistService.STATE_NOT_BLOCKED) { if (state == Ci.nsIBlocklistService.STATE_OUTDATED) { @@ -1015,7 +1015,7 @@ Blocklist.prototype = { continue; if (addon.item instanceof Ci.nsIPluginTag) - addon.item.disabled = true; + addon.item.enabledState = Ci.nsIPluginTag.STATE_DISABLED; else addon.item.softDisabled = true; } diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_bug455906.js b/toolkit/mozapps/extensions/test/xpcshell/test_bug455906.js index 61753d174230..8d5c1c2003a9 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_bug455906.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_bug455906.js @@ -75,37 +75,43 @@ var PLUGINS = [{ // Tests how the blocklist affects a disabled plugin name: "test_bug455906_1", version: "5", - disabled: true, + enabledState : Ci.nsIPluginTag.STATE_DISABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, blocklisted: false }, { // Tests how the blocklist affects an enabled plugin name: "test_bug455906_2", version: "5", - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, blocklisted: false }, { // Tests how the blocklist affects an enabled plugin, to be disabled by the notification name: "test_bug455906_3", version: "5", - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, blocklisted: false }, { // Tests how the blocklist affects a disabled plugin that was already warned about name: "test_bug455906_4", version: "5", - disabled: true, + enabledState: Ci.nsIPluginTag.STATE_DISABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, blocklisted: false }, { // Tests how the blocklist affects an enabled plugin that was already warned about name: "test_bug455906_5", version: "5", - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, blocklisted: false }, { // Tests how the blocklist affects an already blocked plugin name: "test_bug455906_6", version: "5", - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, blocklisted: true }]; @@ -381,8 +387,8 @@ function check_test_pt2() { // Back to starting state addons[2].userDisabled = false; addons[5].userDisabled = false; - PLUGINS[2].disabled = false; - PLUGINS[5].disabled = false; + PLUGINS[2].enabledState = Ci.nsIPluginTag.STATE_ENABLED; + PLUGINS[5].enabledState = Ci.nsIPluginTag.STATE_ENABLED; restartManager(); gNotificationCheck = null; gTestCheck = run_test_pt3; @@ -488,7 +494,7 @@ function check_test_pt3() { function run_test_pt4() { AddonManager.getAddonByID(ADDONS[4].id, function(addon) { addon.userDisabled = false; - PLUGINS[4].disabled = false; + PLUGINS[4].enabledState = Ci.nsIPluginTag.STATE_ENABLED; restartManager(); check_initial_state(function() { gNotificationCheck = check_notification_pt4; diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_duplicateplugins.js b/toolkit/mozapps/extensions/test/xpcshell/test_duplicateplugins.js index e6f2c8cf291c..a7d3cb54a79d 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_duplicateplugins.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_duplicateplugins.js @@ -2,6 +2,8 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ +const Ci = Components.interfaces; + // This verifies that duplicate plugins are coalesced and maintain their ID // across restarts. @@ -10,14 +12,16 @@ var PLUGINS = [{ description: "A duplicate plugin", version: "1", blocklisted: false, - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, filename: "/home/mozilla/.plugins/dupplugin1.so" }, { name: "Duplicate Plugin 1", description: "A duplicate plugin", version: "1", blocklisted: false, - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, filename: "", filename: "/usr/lib/plugins/dupplugin1.so" }, { @@ -25,14 +29,16 @@ var PLUGINS = [{ description: "Another duplicate plugin", version: "1", blocklisted: false, - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, filename: "/home/mozilla/.plugins/dupplugin2.so" }, { name: "Duplicate Plugin 2", description: "Another duplicate plugin", version: "1", blocklisted: false, - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, filename: "", filename: "/usr/lib/plugins/dupplugin2.so" }, { @@ -40,14 +46,16 @@ var PLUGINS = [{ description: "Not a duplicate plugin", version: "1", blocklisted: false, - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, filename: "/home/mozilla/.plugins/dupplugin3.so" }, { name: "Non-duplicate Plugin", // 4 description: "Not a duplicate because the descriptions are different", version: "1", blocklisted: false, - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, filename: "", filename: "/usr/lib/plugins/dupplugin4.so" }, { @@ -55,7 +63,8 @@ var PLUGINS = [{ description: "Not a duplicate plugin", version: "1", blocklisted: false, - disabled: false, + enabledState: Ci.nsIPluginTag.STATE_ENABLED, + get disabled() this.enabledState == Ci.nsIPluginTag.STATE_DISABLED, filename: "/home/mozilla/.plugins/dupplugin5.so" }];