Switch the blocklist service to use the new APIs

VERIFIED FIXED in mozilla1.9.3a5

Status

()

Toolkit
Add-ons Manager
P1
normal
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

Trunk
mozilla1.9.3a5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rewrite])

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Should be pretty straightforward just need a method on AddonManagerPrivate to tell it to update the blocklistState for all add-ons.
(Assignee)

Comment 1

7 years ago
Created attachment 439592 [details] [diff] [review]
patch rev 1

Switches the blocklist service to the new APIs and migrates the tests over for it. It looks like they cover everything I could think of already.

Added a new API to the private add-ons manager, updateAddonAppDisabledStates which makes all providers check and update appDisabled for their add-ons in response to any changes.
Attachment #439592 - Flags: review?(robert.bugzilla)
(Assignee)

Comment 2

7 years ago
Landed on the project branch: http://hg.mozilla.org/projects/addonsmgr/rev/8221fa8f0870
Status: NEW → ASSIGNED
Flags: in-testsuite+
Flags: in-litmus-
Whiteboard: [rewrite] → [rewrite][fixed-in-addonsmgr][needs-review]
Comment on attachment 439592 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/AddonManager.jsm b/toolkit/mozapps/extensions/AddonManager.jsm
>--- a/toolkit/mozapps/extensions/AddonManager.jsm
>+++ b/toolkit/mozapps/extensions/AddonManager.jsm
>@@ -361,16 +361,27 @@ var AddonManagerInternal = {
>    */
>   notifyAddonChanged: function AMI_notifyAddonChanged(aId, aType, aPendingRestart) {
>     this.providers.forEach(function(provider) {
>       callProvider(provider, "addonChanged", null, aId, aType, aPendingRestart);
>     });
>   },
> 
>   /**
>+   * Notifies all providers they need to update the appDisabled property for
>+   * their add-ons in response to some application change such as a blocklist
>+   * update.
nit: s/to some application/to an application/

>+   */
>+  updateAddonAppDisabledStates: function AMI_updateAddonAppDisabledStates() {
>+    this.providers.forEach(function(provider) {
>+      callProvider(provider, "updateAddonAppDisabledStates");
>+    });
>+  },
Comment on attachment 439592 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/nsBlocklistService.js b/toolkit/mozapps/extensions/nsBlocklistService.js
>--- a/toolkit/mozapps/extensions/nsBlocklistService.js
>+++ b/toolkit/mozapps/extensions/nsBlocklistService.js
>...
>@@ -774,105 +775,124 @@ Blocklist.prototype = {
>     }
> 
>     return Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
>   },
> 
>   _blocklistUpdated: function(oldAddonEntries, oldPluginEntries) {
>     var addonList = [];
> 
>-    var em = Cc["@mozilla.org/extensions/manager;1"].
>-             getService(Ci.nsIExtensionManager);
>-    var addons = em.updateAndGetNewBlocklistedItems();
>+    var self = this;
>+    AddonManager.getAddonsByTypes(["extension", "theme", "locale"], function(addons) {
> 
nit: extra newline

Seems like this should just use getAllAddons and filter out plugins or call getAllAddons and have a single loop handle both. Perhaps a followup?

>-    for (let i = 0; i < addons.length; i++) {
>-      let oldState = -1;
>-      if (oldAddonEntries)
>-        oldState = this._getAddonBlocklistState(addons[i].id, addons[i].version,
>-                                                oldAddonEntries);
>-      let state = this.getAddonBlocklistState(addons[i].id, addons[i].version);
>-      // We don't want to re-warn about items
>-      if (state == oldState)
>-        continue;
>+      for (let i = 0; i < addons.length; i++) {
>+        let oldState = Ci.nsIBlocklistService.STATE_NOTBLOCKED;
>+        if (oldAddonEntries)
>+          oldState = self._getAddonBlocklistState(addons[i].id, addons[i].version,
>+                                                  oldAddonEntries);
>+        let state = self.getAddonBlocklistState(addons[i].id, addons[i].version);
> 
>-      addonList.push({
>-        name: addons[i].name,
>-        version: addons[i].version,
>-        icon: addons[i].iconURL,
>-        disable: false,
>-        blocked: state == Ci.nsIBlocklistService.STATE_BLOCKED,
>-        item: addons[i]
>-      });
>-    }
>+        LOG("Blocklist state for " + addons[i].id + " changed from " + oldState + " to " + state);
nitty nit: the add-on blocklist state won't always change will it?
Comment on attachment 439592 [details] [diff] [review]
patch rev 1

r=me. I only scanned the tests since they were mostly wrapping existing code causing whitespace changes.
Attachment #439592 - Flags: review?(robert.bugzilla) → review+
Whiteboard: [rewrite][fixed-in-addonsmgr][needs-review] → [rewrite][fixed-in-addonsmgr]
I just had the Blocklist warning box pop up, saying that some of my add-ons are known to cause stability or security problems.

But the list was empty.

Looking at the actual list( https://www.mozilla.com/en-US/blocklist/ ), I think my version of the Java Deployment Toolkit actually is in the blocklist, but the warning for bad add-ons was empty.

After Restarting Firefox, it doesn't look like anything was actually disabled.

Is this related to this bug? I'm running the latest trunk build with the new EM code.
(Assignee)

Comment 7

7 years ago
(In reply to comment #6)
> Is this related to this bug? I'm running the latest trunk build with the new EM
> code.

Probably caused by this bug yes, please can you file a new bug report for what you're seeing.

Updated

7 years ago
Blocks: 563168
(Assignee)

Comment 8

7 years ago
http://hg.mozilla.org/mozilla-central/rev/bb4c7abafcf7
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Whiteboard: [rewrite][fixed-in-addonsmgr] → [rewrite]
Target Milestone: --- → mozilla1.9.3a5
Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100517 Minefield/3.7a5pre (.NET CLR 3.5.30729)

I have installed the MetaStream 3 Plugin (npViewpoint.dll) which is on the blocklist. After running Firefox for a while I got the blocklisting dialog and the plugin has been disabled.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

7 years ago
Duplicate of this bug: 553378
You need to log in before you can comment on or make changes to this bug.