emit event before loading content scripts

RESOLVED FIXED

Status

Add-on SDK
General
P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: erikvold, Assigned: erikvold)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Assignee: nobody → evold
Created attachment 8599561 [details] [review]
https://github.com/mozilla/addon-sdk/pull/1954

This is ugly but I think it might provide a quick path to implementing the chrome.extension.getBackgroundPage() api. 

I need some help though because I am running into a permission error and if there is a way around it or what the best way to do that would be.

Any suggestions?  I feel like I'm just missing a small change to get all of my tests working in the pull request.
Attachment #8599561 - Flags: feedback?(wmccloskey)
Attachment #8599561 - Flags: feedback?(gkrizsanits)
Comment on attachment 8599561 [details] [review]
https://github.com/mozilla/addon-sdk/pull/1954

ah nvm I think I figured this out, I was using a Page-Worker for a url that dne :/
Attachment #8599561 - Flags: feedback?(wmccloskey)
Attachment #8599561 - Flags: feedback?(gkrizsanits)
Comment on attachment 8599673 [details] [review]
https://github.com/mozilla/addon-sdk/pull/1955

Sorry for the lag here I was supposed to get to this earlier.

I've added a few comments, but I think it's a very nice patch. I prefer the defineProperty version that I noticed only after my comment. After thinking it over I don't think using Cu.isProxy necessary. So you can ignore that comment.

If I recall this code right, then this API is supposed to be used only for internal pages of the add-on (so we don't want to export things to web content)
If I'm wrong then add-on reviewers should be warned what to look for, and maybe a short guide about what not to export would be nice (port for example). And maybe a less generic name...
Attachment #8599673 - Flags: review?(gkrizsanits) → review+
Priority: -- → P1
Blocks: 1161828
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #4)
> Comment on attachment 8599673 [details] [review]
> https://github.com/mozilla/addon-sdk/pull/1955
> 
> Sorry for the lag here I was supposed to get to this earlier.
> 
> I've added a few comments, but I think it's a very nice patch. I prefer the
> defineProperty version that I noticed only after my comment. After thinking
> it over I don't think using Cu.isProxy necessary. So you can ignore that
> comment.
> 
> If I recall this code right, then this API is supposed to be used only for
> internal pages of the add-on (so we don't want to export things to web
> content)

Yes this is correct, I've added some tests to make this more clear now too.
Hmm after adding some more tests which all worked on osx, I am getting some strange errors on traivs:

https://github.com/mozilla/addon-sdk/pull/1955
https://travis-ci.org/mozilla/addon-sdk/jobs/61476077


TEST-START | ./test/test-addon-extras.test addon extras are added to addon uris in panels
TEST-PASS | ./test/test-addon-extras.test addon extras are added to addon uris in panels | no extras
JPM undefined 2015-05-06 14:50:39: stackwalker.cc:125: INFO: Couldn't load symbols for: /lib/x86_64-linux-gnu/libc.so.6|38EBB357B68F03FE6D1E708259DACA7F0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c0a0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f99ffffffff

JPM undefined 2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x5
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f995e0c0ac0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c0d0
2015-05-06 14:50:39: stackwalker.cc:125: INFO: Couldn't load symbols for: /home/travis/build/mozilla/firefox/libxul.so|CDE568CA11D550FA1C3225CFEF0537420
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c7f0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c7f0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0xffffffff
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f995e0c0ac0
2015-05-06 14:50:39: stackwalker.cc:125: INFO: Couldn't load symbols for: /lib/x86_64-linux-gnu/libglib-2.0.so.0|762482351B551C9577B8823CA6B2C7760
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c7f0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0xffffffff7fffffff
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c7f0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996dd90a90
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996dd90a90
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c150
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c180
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x28b5543c
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996dd90a90
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c1e0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c100
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x80ccb800
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c950
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f0100000014
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c270
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c950
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996dd90a98
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c260

JPM undefined 2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c210
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1007fff19f1c2a0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9955050930
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980ca18e0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c978
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c2a0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f997300a360
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980ca18e0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c290
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c290
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980ca18c0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980ca18c0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c2e0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996dd90a90
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c9c950
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c679
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c310
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f0000000001
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f997300a360
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c330
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996dd9f9c0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c350
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c4b0

JPM undefined 2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c19000
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff00000000
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1001100000024
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f997308f400
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c3a0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996dd9f9c0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x3036303530353130
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x100000009
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c3e0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x1001100000022
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f99681747c0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c3e0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f996949be40
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x720074002f0065
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f99694d5c58
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x500000005
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x6d002f0064006c
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x6c0069007a006f
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x66002f0061006c
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x66006500720069
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f997307da60
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f0036393034
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c490
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c480
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f997304d110
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c720
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c786a8
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c4b0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c510
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7f9980c782e8

JPM undefined 2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c500
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x0
2015-05-06 14:50:39: basic_code_modules.cc:88: INFO: No module at 0x7fff19f1c660


I need to figure out what the issue is here..
That looks kinda like a hang.
(In reply to Bill McCloskey (:billm) from comment #7)
> That looks kinda like a hang.

Ah right, I'm having trouble showing panels on travis for some reason, I'll have to rewrite the tests to avoid this.
Created attachment 8602323 [details] [review]
https://github.com/mozilla/addon-sdk/pull/1966

I've made a small change to the content/sandbox.js file to remove the unnecessary use of `wrapReflectors` and I added some tests to confirm that the `extras` variable is undefined for non add-on content.

We could use more tests, but I'd like to add those in follow-up bugs if that is alright, they should be easier to review so that I can get others help out.
Attachment #8602323 - Flags: review?(gkrizsanits)
(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #9)
> Created attachment 8602323 [details] [review]
> https://github.com/mozilla/addon-sdk/pull/1966

Not a big github guru myself... is there a way to see an interdiff between the
last version and this one? It's quite a long patch to check against the last one
line by line, and don't want to re-read each tests if I don't have too...

> 
> I've made a small change to the content/sandbox.js file to remove the
> unnecessary use of `wrapReflectors` and I added some tests to confirm that
> the `extras` variable is undefined for non add-on content.
> 

I'm still not quite sure what APIs you want to export this way. I thought you
might need wrapReflectors for exporting functions that can take native arguments.
(like DOM nodes), but I've just double checked it and we don't do cloning for arguments
any longer so you probably don't need wrapReflectors. What I'm particularly interested is
arguments that have callable properties / are callables themselves.

The real question is though, that why do we need cloning here. If we want this feature only for
trusted pages from the addon itself and we're in the requiresAddonGlobal branch (I think
your isAddonContent check unnecessary after requiresAddonGlobal by the way) that means the window
has system principal and the worker too. So exporting things should be plain and simple. Or do we have any cases where we run an internal addon page with content privileges? I don't think so because then
the addon property we define in the next line would likely not work either. So if you try all this, without any cloning, what fails exactly that you're trying to work around with cloning?
After thinking about this some more today, I realize that it will be better to just emit a system event when a sandbox for a content script is created, at which point any add-on could inject things into the window or the sandbox.  I think this method will be easier to implement, test, and understand than the sdk/addon/extras module I was working on so I'm going to ditch that patch.
Summary: Add experimental module to add things to the addon global object → emit system emit before loading content scripts
Attachment #8602323 - Flags: review?(gkrizsanits)
Summary: emit system emit before loading content scripts → emit event before loading content scripts
Created attachment 8603741 [details] [review]
https://github.com/mozilla/addon-sdk/pull/1968
Attachment #8599561 - Attachment is obsolete: true
Attachment #8599673 - Attachment is obsolete: true
Attachment #8602323 - Attachment is obsolete: true
Attachment #8603741 - Flags: review?(jsantell)
Duplicate of this bug: 1158330
Comment on attachment 8603741 [details] [review]
https://github.com/mozilla/addon-sdk/pull/1968

FWIW, I see that log spam anywhere, definitely not from this patch
Attachment #8603741 - Flags: review?(jsantell) → review+

Comment 15

3 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/8fd686f13da7060cee7e43d24e739f3a24caafc8
Bug 1159619 emit event before loading content scripts

https://github.com/mozilla/addon-sdk/commit/e9128c08e1dac8646f7ff47b121caf0afd5f688f
Merge pull request #1968 from erikvold/1159619v5

Bug 1159619 emit event before loading content scripts r=@jsantell
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.