Closed
Bug 897735
Opened 11 years ago
Closed 10 years ago
Support regular expression filters for metadata in extension blocks
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: jorgev, Assigned: sachin)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [squeaky][engineering-friend])
Attachments
(1 file, 8 obsolete files)
61.61 KB,
patch
|
Details | Diff | Splinter Review |
We only support blocking extensions by ID at the moment, and that has forced us to write Add-ons Manager patches to deal with add-ons that are distributed with multiple IDs (like on bug 799266). We should add support for more advanced blocks for extensions. What we want is filter by add-on name and add-on creator, using regular expressions. The current add-on ID filter already supports regular expressions, so there's probably some code there than can be reused. If more than one of the 3 filters is used, all 3 must match for an add-on to be blocked. Here are examples of what I think these blocks should look like. Only ID filter: > <emItem blockID="XXX" id="/^pink@.*\.info$/"> > <versionRange minVersion="0" maxVersion="*" severity="3"> > </versionRange> > </emItem> Only name filter: > <emItem blockID="XXX" name="/^Mozilla Corp\.$/"> > <versionRange minVersion="0" maxVersion="*" severity="1"> > </versionRange> > </emItem> All filters: > <emItem blockID="XXX" id="/^pink@.*\.info$/ name="/^Mozilla Corp\.$/" > creator="/evild00d/"> > <versionRange minVersion="0" maxVersion="*" severity="1"> > </versionRange> > </emItem> Since we already support regular expressions in the ID, I suppose the code already handles multiple matches, but it'd be good to verify this is the case.
Assignee | ||
Comment 1•11 years ago
|
||
This needs changes to the nsIBlocklistService interface. An add-on's ID is no longer sufficient to decide if it is blocklisted or not. I will need to change everything that uses this component. I hope that is okay.
Flags: needinfo?
Reporter | ||
Comment 2•11 years ago
|
||
But passing the ID is sufficient information, no? On the implementation side you can get whatever data is necessary to determine if the add-on is blocklisted. So, I don't think it's necessary to change the interface.
Flags: needinfo?
Assignee | ||
Comment 3•11 years ago
|
||
Ah, sorry. :( I should have realized.
Assignee | ||
Comment 4•11 years ago
|
||
So I tried to use AddonManager API to retrieve the name and creator from the ID. But the problem is that getAddonByID works asynchronously. A call to getAddonBlocklistState here: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js#903 is where it is decided if an add-on is blocklisted. But getAddonBlocklistState is part of nsIBlocklistService. If we don't want to alter the interface, only at some point inside getAddonBlocklistState should we fetch the add-on's creator and name. But getAddonByID is asynchronous. Since getAddonByID takes time, getBlocklistState proceeds to return, not waiting for the name and creator to be fetched. I'm left to think that changing getAddonBlocklistState (to make it accept the name and creator as params) might be necessary. But this means changing nsIBlocklistService. Or maybe we should cache the name and creator information in the object for use by getAddonBlocklistState. What do you think?
Assignee | ||
Comment 5•11 years ago
|
||
The add-on is stored in _addonCache for use while checking.
Comment 6•11 years ago
|
||
I think we should also be able to block by updateURL, and probably homepage.
Reporter | ||
Comment 7•11 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #4) > Or maybe we should cache the name and creator information in the object for > use by getAddonBlocklistState. > > What do you think? Maybe we just need to cache the add-on ID and blocklist state in the object. I assume the blocklist service knows this information every time the blocklist is read and updated. If we were forced to change the service API, it would probably make more sense to make it asynchronous too, but I don't think we need to for now. (In reply to Kris Maglione [:kmag] from comment #6) > I think we should also be able to block by updateURL, and probably homepage. I agree with this. Let's include them on this bug.
Summary: Support regular expression filters for "name" and "creator" in extension blocks → Support regular expression filters for metadata in extension blocks
Assignee | ||
Updated•11 years ago
|
Attachment #783234 -
Flags: review?(jorge)
Reporter | ||
Comment 8•11 years ago
|
||
Comment on attachment 783234 [details] [diff] [review] cache version We discussed the possible approaches for this and agreed that this was the least disruptive one. This is still missing Kris's suggested additions, and of course a review by someone who better understands this code :)
Attachment #783234 -
Flags: review?(jorge) → review+
Assignee | ||
Comment 9•11 years ago
|
||
The idea of this patch is to cache (in the constructor of the blocklist service constructor) all the add-ons installed so that when _findMatchingAddonEntry is called with just the ID of the add-on, it can use the initially created cache to get the other attributes of the add-on (like creator, name etc.). But this doesn't seem to work well either. The AddonManager uses the blocklist service on startup. What I noticed is, the cache isn't created yet when the AddonManager is using the blocklist service. This leads to incorrect blocklist states getting fetched. This is again probably because the cache is loaded asynchronously. Jorge, the other solution you told me on IRC and in comment 7 also leads to the same issue since that also involves creating a map using AddonManager's asynchronous API. Is changing the blocklist service into an asynchronous API the only way?
Attachment #783234 -
Attachment is obsolete: true
Updated•11 years ago
|
Whiteboard: [squeaky] → [squeaky][engineering-friend]
Reporter | ||
Comment 10•11 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #9) > Jorge, the other solution you told me on IRC and in comment 7 also leads to > the same issue since that also involves creating a map using AddonManager's > asynchronous API. My suggestion from comment #7 is that we populate a local cache on the blocklist service when it is run, and all we need to store are the add-on ID and its blocklist status. I don't really understand why this can't be accomplished without additional calls to the Add-ons Manager.
Assignee | ||
Comment 11•11 years ago
|
||
Additional calls to the Add-ons Manager aren't necessary in both my patch and what you suggest. What we need is just an initial call as you say. But this initial call is itself causing problems. I think there are 2 problems in what you suggest: 1. Some add-ons may be blocked by a combination of attributes that excludes add-on ID. In this case, we won't be able to store the add-on's ID in the cache. The workaround you had suggested where we search for installed add-ons that match the blocklist attributes and find the IDs, will need to use the Add-on Manager API. 2. If a to-be-blocked add-on is installed after the blocklist is read, the add-on's ID will not be ready in the cache if the blocklist entry didn't contain the add-on's ID as an attribute of an entry. (This problem exists in my patch as well.) In the patch, I build an add-on cache containing installed add-ons in the constructor of the service. (This can't be done only when the blocklist file is updated, because a browser restart between blocklist updates will empty the cache.) This action uses the Add-on Manager API which is causing problems. Hope I'm not missing something.
Reporter | ||
Comment 12•11 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #11) > What we need is just an initial call as you say. But > this initial call is itself causing problems. Which initial call? When is this needed? > 1. Some add-ons may be blocked by a combination of attributes that excludes > add-on ID. In this case, we won't be able to store the add-on's ID in the > cache. All I'm saying is that we should have a mapping that has <add-on ID, blocklist status>. That's all we need to know in order for the current API to continue working. This doesn't need to be related to any other data structures we use internally. > 2. If a to-be-blocked add-on is installed after the blocklist is read, the > add-on's ID will not be ready in the cache if the blocklist entry didn't > contain the add-on's ID as an attribute of an entry. (This problem exists in > my patch as well.) Then let's just trigger a blocklist read when an add-on is installed, regardless of whether we use your approach or mine. Clearly the rules are much more complex now and going through an ID list won't suffice.
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Jorge Villalobos [:jorgev] from comment #12) > (In reply to Sachin Hosmani [:sachin] from comment #11) > > What we need is just an initial call as you say. But > > this initial call is itself causing problems. > > Which initial call? When is this needed? I was referring to the AddonManager calls that will get the add-on's ID from other properties in your approach (to build the map) and vice versa in mine, each of which are necessary in their respective cases. > > 1. Some add-ons may be blocked by a combination of attributes that excludes > > add-on ID. In this case, we won't be able to store the add-on's ID in the > > cache. > > All I'm saying is that we should have a mapping that has <add-on ID, > blocklist status>. That's all we need to know in order for the current API > to continue working. This doesn't need to be related to any other data > structures we use internally. I assume you are suggesting we build this map when the blocklist service parses the blocklist file. So consider a blocklist entry with out an add-on's ID specified. How do we add this add-on to the map? We could fetch the add-on's ID from the AddonManager but that will be an asynchronous call. > > 2. If a to-be-blocked add-on is installed after the blocklist is read, the > > add-on's ID will not be ready in the cache if the blocklist entry didn't > > contain the add-on's ID as an attribute of an entry. (This problem exists in > > my patch as well.) > > Then let's just trigger a blocklist read when an add-on is installed, > regardless of whether we use your approach or mine. Clearly the rules are > much more complex now and going through an ID list won't suffice. You mean to say that an add-on ID can't remain the sole attribute in the blocklist file that decides an add-on's blocklist state, right?
Comment 14•11 years ago
|
||
Is there a particular reason to work so hard to keep the existing API? Seems like this would be far easier to just change it so you pass the addon object with all its properties there for synchronous access.
Reporter | ||
Comment 15•11 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #13) > > All I'm saying is that we should have a mapping that has <add-on ID, > > blocklist status>. That's all we need to know in order for the current API > > to continue working. This doesn't need to be related to any other data > > structures we use internally. > > I assume you are suggesting we build this map when the blocklist service > parses the blocklist file. So consider a blocklist entry with out an > add-on's ID specified. How do we add this add-on to the map? We could fetch > the add-on's ID from the AddonManager but that will be an asynchronous call. Okay, but that would happen while the blocklist is being read, which is not a problem as far as I know. The sync/async problem is when someone is calling the blocklist service and asking if a particular add-on is blocklisted. > > > 2. If a to-be-blocked add-on is installed after the blocklist is read, the > > > add-on's ID will not be ready in the cache if the blocklist entry didn't > > > contain the add-on's ID as an attribute of an entry. (This problem exists in > > > my patch as well.) > > > > Then let's just trigger a blocklist read when an add-on is installed, > > regardless of whether we use your approach or mine. Clearly the rules are > > much more complex now and going through an ID list won't suffice. > > You mean to say that an add-on ID can't remain the sole attribute in the > blocklist file that decides an add-on's blocklist state, right? Well, not really. I mean, that last statement is true, we will no longer have IDs for all add-on blocks. But what I was trying to say is that the internal list of IDs in the blocklist service will not suffice anymore. So, when a new add-on is installed, we will need more in order to determine its blocklist state. (In reply to Dave Townsend (:Mossop) from comment #14) > Is there a particular reason to work so hard to keep the existing API? Seems > like this would be far easier to just change it so you pass the addon object > with all its properties there for synchronous access. I thought it'd be better for compatibility purposes, since it seemed to be an unnecessary change. But if things are getting so complicated (and I guess they are), we should consider this approach. Passing the addon object would be cleaner than passing individual attributes, for sure. If all the callers have that object available, it shouldn't be a problem to switch to that (other than potential add-on compat).
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to Jorge Villalobos [:jorgev] from comment #15) > Okay, but that would happen while the blocklist is being read, which is not > a problem as far as I know. Um, I think it is a problem. A line of async code in the midst of synchronously executing code seems wrong to me, since the data that is retrieved from the async call is needed.
Comment 17•11 years ago
|
||
(In reply to Jorge Villalobos [:jorgev] from comment #15) > I thought it'd be better for compatibility purposes, since it seemed to be > an unnecessary change. But if things are getting so complicated (and I guess > they are), we should consider this approach. Passing the addon object would > be cleaner than passing individual attributes, for sure. If all the callers > have that object available, it shouldn't be a problem to switch to that > (other than potential add-on compat). I see a few add-ons on AMO doing the right thing and checking the blocklistState property to check it, none using nsIBlocklistService though (aside from the constants there). Given that we already do additional blocklist checks outside of nsIBlocklistService then if they're using that they're not getting the correct answer so I don't think we need to avoid breaking this.
Assignee | ||
Comment 18•11 years ago
|
||
- Shall I remove the hard-coded block entries added in bug 799266? - Since updateURL isn't a property of AddonWrapper I'm not sure of how to get that property. While we could pass AddonInternal objects to blocklist service methods, the blocklist service itself uses AddonManager to iterate through all the installed add-ons when the blocklist gets updated and AddonManager returns AddonWrapper objects. (http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js#896)
Flags: needinfo?(jorge)
Reporter | ||
Comment 19•11 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #18) > - Shall I remove the hard-coded block entries added in bug 799266? No, I think we should leave them in, since we want to maximize their coverage, and only new users will benefit from these changes.
Flags: needinfo?(jorge)
Assignee | ||
Comment 20•11 years ago
|
||
Could you please suggest a workaround for the other question in comment 18?
Flags: needinfo?(dtownsend+bugmail)
Comment 21•11 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #20) > Could you please suggest a workaround for the other question in comment 18? I don't see a reason why we couldn't expose updateURL. The question becomes whether we expose exactly what is in the install.rdf or we first do the substitutions from http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#5603. I'm leaning towards the latter but I'm not sure what our needs are for blocklisting and this is Blair's call really.
Flags: needinfo?(dtownsend+bugmail) → needinfo?(bmcbride)
Assignee | ||
Comment 22•11 years ago
|
||
Is the the purpose of blocklist API to find blocklist information of only installed add-ons or also other add-ons (ones not installed)? I mean to ask if it is used to simply read the contents of the blocklist file. This fix won't work for that kind of usage since we can't get an add-on object for add-ons that aren't installed. In test_bug393285.js, I think the API is used on add-ons that aren't installed. What do I do about such tests?
Assignee | ||
Comment 23•11 years ago
|
||
Some tests like in comment 22 failed.
Attachment #788499 -
Attachment is obsolete: true
Comment 24•11 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #22) > Is the the purpose of blocklist API to find blocklist information of only > installed add-ons or also other add-ons (ones not installed)? I mean to ask > if it is used to simply read the contents of the blocklist file. This fix > won't work for that kind of usage since we can't get an add-on object for > add-ons that aren't installed. As long as it works for installed or about to be installed add-ons then I think that that is all that matters. > In test_bug393285.js, I think the API is used on add-ons that aren't > installed. > What do I do about such tests? Just create mock Addon objects for those
Comment 25•11 years ago
|
||
(In reply to Dave Townsend (:Mossop) from comment #21) > (In reply to Sachin Hosmani [:sachin] from comment #20) > > Could you please suggest a workaround for the other question in comment 18? > > I don't see a reason why we couldn't expose updateURL. The question becomes > whether we expose exactly what is in the install.rdf or we first do the > substitutions Eh, simple = good. Lets just go with the value from install.rdf. Just exposing the raw URL from install.rdf is far simpler, and that's all we need for this really. And other consumer is probably only going to want the domain. There's also the question of if we did do the substitutions, what would UPDATE_TYPE be substituted for?
Flags: needinfo?(bmcbride)
Comment 26•11 years ago
|
||
Comment on attachment 808207 [details] [diff] [review] supports attributes except updateURL Review of attachment 808207 [details] [diff] [review]: ----------------------------------------------------------------- *Much* prefer this approach. I've previously stated that any change that requires the API to become async just has to wait for the blocklist to be rewritten (bug 782261) - because switching to async APIs is just too big of a change (and too regression prone) for a codebase we want to throw away soon anyway.
Attachment #808207 -
Flags: feedback+
Assignee | ||
Comment 27•11 years ago
|
||
I've exposed updateURL as it is in install.rdf. I also fixed the old tests that are affected by the change in API.
Attachment #808207 -
Attachment is obsolete: true
Attachment #819814 -
Flags: review?(bmcbride)
Comment 28•11 years ago
|
||
Comment on attachment 819814 [details] [diff] [review] supports all attributes Review of attachment 819814 [details] [diff] [review]: ----------------------------------------------------------------- Looks good :) One last remaining issue: There are a few other areas of the tree in both /toolkit/ and /browser/ that access these APIs - need to update those too so we don't break things. eg, SocialService.jsm, aboutDialog.js, etc. Also, we'll need a bug filed to ensure someone checks Thunderbird and Seamonkey for usage of these APIs. ::: toolkit/mozapps/extensions/nsBlocklistService.js @@ +766,4 @@ > versions: [], > prefs: [], > + blockID: null, > + attributes : {} Since this used to hold data we loop over, it should be a Map. @@ +767,5 @@ > prefs: [], > + blockID: null, > + attributes : {} > + // Atleast one of these must get added to attributes: > + // id, creator, name, updateURL, homepageURL Nit: Reduce maintenance, don't explicitly list the properties - just mention EXTENSION_BLOCK_FILTERS. @@ +772,4 @@ > }; > > + // Add-on attributes don't start with '/', so any of those > + // starting with '/' must be a regex. Nit: This is only technically true for IDs. So say here that any filters that start with '/' are just interpreted as a regex. And if we need to match an attribute value starting with '/', the filter needs to do so via a regex. @@ +772,5 @@ > }; > > + // Add-on attributes don't start with '/', so any of those > + // starting with '/' must be a regex. > + function RegExpCheck(attr) { Nit: Code style. Functions don't start with a capital letter.
Attachment #819814 -
Flags: review?(bmcbride) → review-
Reporter | ||
Comment 29•11 years ago
|
||
Sachin, will you be able to continue working on this?
Flags: needinfo?(sachinhosmani2)
Assignee | ||
Comment 30•11 years ago
|
||
Yes I will. I can get to it in about a week. I got busy because of exams and college work.
Flags: needinfo?(sachinhosmani2)
Assignee | ||
Comment 31•10 years ago
|
||
In here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/social/SocialService.jsm#592 , a blocklist state check is done by ID read from the manifest. But that is synchronously executing code. Since there is no callback passed there, we can't get an add-on object with the Add-on Manager API. I'm thinking we could just create dummy add-on objects with just whatever we need that is in the manifest : [https://developer.mozilla.org/en-US/docs/Social_API/Manifest] and pass it to the blocklist service. We can get everything but updateURL from the manifest as I see it. Will this be ok?
Flags: needinfo?(bmcbride)
Comment 32•10 years ago
|
||
I'm not familiar with SocialService.jsm, so better to see what Shane thinks. Shane: nsIlocklistService.getAddonBlocklistState() is changing from accepting an addon ID and version string to taking an Addon object. Sachin mentions in comment 31 that this might be a bit awkward for the case he identified. Thoughts?
Flags: needinfo?(scaraveo)
Comment 33•10 years ago
|
||
(In reply to Blair McBride [:Unfocused] from comment #32) > I'm not familiar with SocialService.jsm, so better to see what Shane thinks. > > Shane: nsIlocklistService.getAddonBlocklistState() is changing from > accepting an addon ID and version string to taking an Addon object. Sachin > mentions in comment 31 that this might be a bit awkward for the case he > identified. Thoughts? There are a couple places that need to be updated. IIUC we just need to create an instance of AddonWrapper (Also in SocialService.jsm) passing the manifest as the argument. .getAddonBlocklistState(new AddonWrapper(data), ...) Am I reading correctly that the api is remaining synchronous? If not, there is a bit more effort to make this work.
Flags: needinfo?(scaraveo)
Comment 34•10 years ago
|
||
I think these changes would do it for socialapi, I haven't tested this though.
Assignee | ||
Comment 36•10 years ago
|
||
Made those changes.
Attachment #819814 -
Attachment is obsolete: true
Attachment #8349982 -
Flags: review?(bmcbride)
Updated•10 years ago
|
Flags: needinfo?(bmcbride)
Comment 37•10 years ago
|
||
Comment on attachment 8349982 [details] [diff] [review] blocklist-regexp.patch Review of attachment 8349982 [details] [diff] [review]: ----------------------------------------------------------------- Good to go! I forget, do you have access to land this yourself? ::: toolkit/mozapps/extensions/nsBlocklistService.js @@ +766,4 @@ > versions: [], > prefs: [], > + blockID: null, > + attributes : new Map() Nit: No space before the colon.
Attachment #8349982 -
Flags: review?(bmcbride) → review+
Updated•10 years ago
|
Attachment #8349568 -
Attachment is obsolete: true
Assignee | ||
Comment 38•10 years ago
|
||
No I don't have access to land this.
Assignee | ||
Comment 39•10 years ago
|
||
Attachment #8349982 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 40•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5b19ca07781b
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [squeaky][engineering-friend] → [squeaky][engineering-friend][fixed-in-fx-team]
Comment 41•10 years ago
|
||
Backed out for mochitest-bc orange. https://hg.mozilla.org/integration/fx-team/rev/1a5a6071b1a3 https://tbpl.mozilla.org/php/getParsedLog.php?id=32583492&tree=Fx-Team
Comment 42•10 years ago
|
||
\o/ Broke the social tests! None of which this test actually touched... *sigh* Any idea what's up, Sachin? Or would would you like Shane to have a look? (Cos I have no idea)
Status: NEW → ASSIGNED
Flags: needinfo?(sachinhosmani2)
Updated•10 years ago
|
Whiteboard: [squeaky][engineering-friend][fixed-in-fx-team] → [squeaky][engineering-friend]
Assignee | ||
Comment 43•10 years ago
|
||
No, I don't understand what is happening either. Shane, can you have a look?
Flags: needinfo?(sachinhosmani2) → needinfo?(mixedpuppy)
Comment 44•10 years ago
|
||
(In reply to Sachin Hosmani [:sachin] from comment #43) > No, I don't understand what is happening either. Shane, can you have a look? I'll do a build with this patch and see what's up.
Comment 45•10 years ago
|
||
patch with socialapi fixes. try at: https://tbpl.mozilla.org/?tree=Try&rev=d8df60540bdf
Attachment #8356014 -
Attachment is obsolete: true
Flags: needinfo?(mixedpuppy)
Comment 47•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/933bf791b773
Keywords: checkin-needed
Whiteboard: [squeaky][engineering-friend] → [squeaky][engineering-friend][fixed-in-fx-team]
Comment 48•10 years ago
|
||
Backed out for completely destroying Windows mochitests. https://hg.mozilla.org/integration/fx-team/rev/716a6ec17cec https://tbpl.mozilla.org/php/getParsedLog.php?id=32978161&tree=Fx-Team This looks relevant: 07:54:32 INFO - *** ERROR addons.xpi: Unable to read add-on manifest from c:\users\cltbld\appdata\local\temp\tmpakyznu\extensions\staged\workerbootstrap-test@mozilla.org: [Exception... "Not enough arguments [nsIBlocklistService.getAddonBlocklistState]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: AddonInternal.prototype.blocklistState :: line 6209" data: no] 07:54:32 INFO - *** ERROR addons.xpi: Unable to read add-on manifest from c:\users\cltbld\appdata\local\temp\tmpakyznu\extensions\staged\worker-test@mozilla.org: [Exception... "Not enough arguments [nsIBlocklistService.getAddonBlocklistState]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: AddonInternal.prototype.blocklistState :: line 6209" data: no] 07:54:32 INFO - *** ERROR addons.xpi: Unable to read add-on manifest from c:\users\cltbld\appdata\local\temp\tmpakyznu\extensions\staged\special-powers@mozilla.org: [Exception... "Not enough arguments [nsIBlocklistService.getAddonBlocklistState]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: AddonInternal.prototype.blocklistState :: line 6209" data: no] 07:54:32 INFO - *** ERROR addons.xpi: Unable to read add-on manifest from c:\users\cltbld\appdata\local\temp\tmpakyznu\extensions\staged\mochikit@mozilla.org: [Exception... "Not enough arguments [nsIBlocklistService.getAddonBlocklistState]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: AddonInternal.prototype.blocklistState :: line 6209" data: no] 07:54:32 INFO - *** ERROR addons.xpi: Unable to read add-on manifest from c:\users\cltbld\appdata\local\temp\tmpakyznu\extensions\staged\indexedDB-test@mozilla.org: [Exception... "Not enough arguments [nsIBlocklistService.getAddonBlocklistState]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: AddonInternal.prototype.blocklistState :: line 6209" data: no]
Flags: in-testsuite+
Whiteboard: [squeaky][engineering-friend][fixed-in-fx-team] → [squeaky][engineering-friend]
Comment 49•10 years ago
|
||
Please make sure this has a green Try run before requesting checkin again.
Comment 50•10 years ago
|
||
Hmm, looks like it was needs-clobber bustage. Re-landed with a CLOBBER touch. Sorry for the churn. https://hg.mozilla.org/integration/fx-team/rev/c8e2cb87f6c1
Comment 51•10 years ago
|
||
I filed bug 959653 for the needs-clobber.
You did not update nsIBlocklistService's IID when changing the interface. That's what caused the build failures. You need to push a followup to do that.
Comment 53•10 years ago
|
||
The issue was a missing IID bump to nsIBlocklistService. https://hg.mozilla.org/integration/fx-team/rev/8c0180723f67
Comment 54•10 years ago
|
||
I'm still seeing this, even after clobber building fx-team tip a couple times in the last hour.
Comment 55•10 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #53) > The issue was a missing IID bump to nsIBlocklistService. > https://hg.mozilla.org/integration/fx-team/rev/8c0180723f67 Aaaand that accidentally un-did all other changes in that file too. Pushed a fix for that: https://hg.mozilla.org/integration/fx-team/rev/869ff4bc5354
Comment 56•10 years ago
|
||
(In reply to Brandon Benvie [:benvie] from comment #54) > I'm still seeing this, even after clobber building fx-team tip a couple > times in the last hour. A build error, or general brokenness that may be accounted for by comment 55?
Comment 57•10 years ago
|
||
(In reply to Blair McBride [:Unfocused] from comment #56) > (In reply to Brandon Benvie [:benvie] from comment #54) > > I'm still seeing this, even after clobber building fx-team tip a couple > > times in the last hour. > > A build error, or general brokenness that may be accounted for by comment 55? Looks like the patch in comment 55 will fix this.
Comment 58•10 years ago
|
||
*sigh* Can I just quit while I'm behind?
https://hg.mozilla.org/mozilla-central/rev/c8e2cb87f6c1 https://hg.mozilla.org/mozilla-central/rev/8c0180723f67 https://hg.mozilla.org/mozilla-central/rev/869ff4bc5354
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Assignee | ||
Updated•10 years ago
|
Keywords: dev-doc-complete
Assignee | ||
Comment 60•10 years ago
|
||
Updated https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIBlocklistService
You need to log in
before you can comment on or make changes to this bug.
Description
•