1529374 - Installing built in addon breaks devtools addons tests

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[Dale Harvey (:daleharvey)]

Looking at the try run @ https://treeherder.mozilla.org/#/jobs?repo=try&revision=9705c1cae7e8b06d0cce47aa517273785c067302&selectedJob=229324147

There are a lot of devtools failures around listing / reading addons that dont seem like they could be caused by my code other than the fact that it installs builtin addons

Comment 1

[Dale Harvey (:daleharvey)]

Attachment: 0001-a-bug.patch

This patch reproduces the bug for me, can see with

/mach test devtools/client/shared/test/browser_dbg_listaddons.js

Comment 2

[Dale Harvey (:daleharvey)]

Hey Andrew sorry I think theres another extensions bug here

Comment 3

[Andrew Swan [:aswan]]

I think something like this is the right approach. There are a bunch of tests, however, that make assumptions about the return value from getResourceURI(). I think updating those tests, rather than trying to put more special-case handling into getResourceURI() is probably ideal. Dale, let me know if you have time to go through the tests or if you'd like me to do it.

--- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -1025,20 +1025,21 @@ AddonWrapper = class {
    *
    * @param {string?} aPath
    *        The path in the add-on to get the URI for or null to get a URI to
    *        the file or directory the add-on is installed as.
    * @returns {nsIURI}
    */
   getResourceURI(aPath) {
     let addon = addonFor(this);
-    if (!aPath)
-      return Services.io.newFileURI(addon._sourceBundle);
-
-    return XPIInternal.getURIForResourceInFile(addon._sourceBundle, aPath);
+    let url = Services.io.newURI(addon.rootURI);
+    if (aPath) {
+      url = Services.io.newURI(aPath, null, url);
+    }
+    return url;
   }
 };
 
 function chooseValue(aAddon, aObj, aProp) {
   let repositoryAddon = aAddon._repositoryAddon;
   let objValue = aObj[aProp];
 
   if (repositoryAddon && aProp in repositoryAddon &&

Comment 4

[Dale Harvey (:daleharvey)]

Hey Andrew

I tried pushing this to try and started seeing a whole bunch of new failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4421817c7fe71d9e2b127f102df20b9881fdcb4c&selectedJob=229678903 (/test_filepointer.js devtools/server/tests/browser/browser_storage_dynamic_windows.js etc look suspicious)

I am not so familiar with this stuff and got a lot of fairly urgent work at the moment so if theres any change you have cycles asap it would be a huge help

Cheers
Dale

Comment 5

[Andrew Swan [:aswan]]

Attachment: Bug 1529374 Fix getResourceURI() for builtin addons

Comment 6

[Andrew Swan [:aswan]]

Dale can you try the attached patch and let me know if it works for you?

Comment 7

[Dale Harvey (:daleharvey)]

Doing a try run @ https://treeherder.mozilla.org/#/jobs?repo=try&revision=03aae059367a39d6df5a0d921c4d25e7e0c489b1, cheers

Comment 8

[Dale Harvey (:daleharvey)]

There is still a few extensions/debugging failures after this patch, but a lot less than without it (https://treeherder.mozilla.org/#/jobs?repo=try&revision=79c8764181d3bc615d2ed5bb43b99851bda09e00&selectedJob=230623630)

So be great to get this landed and can investigate the other failures seperately, thanks

Comment 9

[Andrew Swan [:aswan]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4f9757e41e19aed16bead8cbd9ae533637cc3cc
Bug 1529374 Fix getResourceURI() for builtin addons r=kmag

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f4f9757e41e1

Comment 11

[Madalin Cotetiu]

From my understanding this issue is related to the addon tests. Is there any need of manual QA here? If not can you please mark it as "qe-verify- "