1158238 - [System] Add 'import-app' activity

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Justin D'Arcangelo [:justindarc]]

As suggested here ( https://bugzilla.mozilla.org/show_bug.cgi?id=1123846#c11 ), the System app should support an 'import-app' activity that can check if dev mode is enabled and return an error if not. This is to allow for apps to create add-ons for themselves.

Comment 1

[Kevin Grandon :kgrandon]

I will work on this.

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:bug_1158238_app_import_activity > mozilla-b2g:master

Comment 3

[Justin D'Arcangelo [:justindarc]]

Attachment: addon-custom-events.zip

Test add-on for importing.

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8597376 [details] [review]
[gaia] KevinGrandon:bug_1158238_app_import_activity > mozilla-b2g:master

Hey Justin - Here is part 1, just handling the activity for now. Could I delegate the review to you?

I'm going to file another bug for showing an error if developer mode is not turned on as that might need some UX/strings. We can land this initially so we have something to work with. Thanks!

Comment 5

[[:fabrice] Fabrice Desré]

Keivn, gaia doesn't know if developer mode is set or not because there's no setting. But you don't need to know here because the *caller* will get an onerror when creating the activity (I'm returning SecurityError) so callers need to manage that.

Comment 6

[Kevin Grandon :kgrandon]

(In reply to Fabrice Desré [:fabrice] from comment #5)
> Keivn, gaia doesn't know if developer mode is set or not because there's no
> setting. But you don't need to know here because the *caller* will get an
> onerror when creating the activity (I'm returning SecurityError) so callers
> need to manage that.

Sounds good, then we don't need bug 1158342, but will need to add some UI if we encounter a SecurityError.

Comment 7

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8597376 [details] [review]
[gaia] KevinGrandon:bug_1158238_app_import_activity > mozilla-b2g:master

Kevin, I think we at least need to handle exceptions during `mgmt.import()` with a `postError()` call. I also commented in the PR about possibly using `postMessage()` to pass something useful, but I'm not as concerned about that as I am with the error case.

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8597376 [details] [review]
[gaia] KevinGrandon:bug_1158238_app_import_activity > mozilla-b2g:master

Thanks Justin - I've gone ahead and made the updates. Please take another look when you can.

Comment 9

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8597376 [details] [review]
[gaia] KevinGrandon:bug_1158238_app_import_activity > mozilla-b2g:master

LGTM. Thanks for making those changes!

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8597376 [details] [review]
[gaia] KevinGrandon:bug_1158238_app_import_activity > mozilla-b2g:master

Thanks for the review. R=me as well as I'm delegating this to a non-system peer.

Comment 11

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/7e8cd362728be8241dffb781a7314db0f537e4ae

Comment 12

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:bug_1158238_follow_up_rename_activity > mozilla-b2g:master

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8598824 [details] [review]
[gaia] KevinGrandon:bug_1158238_follow_up_rename_activity > mozilla-b2g:master

Simple follow-up to add a preference and rename the activity. R=me assuming the tests pass.

Comment 14

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/1e7b0df5260664713172cac60cc2df5def1a2a06

Comment 15

[Kevin Grandon :kgrandon]

Had to back this out due to causing bug 1159539. Will take a look at this again.

https://github.com/mozilla-b2g/gaia/commit/320bdae4d4d3e0a84230d04086fe40736b69ec5b
https://github.com/mozilla-b2g/gaia/commit/d85284fd03e9eaffc1cafd83308dec061ee29ee1

Comment 16

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:reland_bug_1158238 > mozilla-b2g:master

Comment 17

[Kevin Grandon :kgrandon]

Comment on attachment 8599552 [details] [review]
[gaia] KevinGrandon:reland_bug_1158238 > mozilla-b2g:master

Alive - wondering if you can take a look at this. It this may have a slight impact for your base module patch, and I can help rebase your patch if necessary. I assume that we may want to lazy load both browser.js and import.js. I think activity_handler needs to be there on startup though for the mozSetMessageHandler call?

Let me know if you have any questions, thanks!

Comment 18

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8599552 [details] [review]
[gaia] KevinGrandon:reland_bug_1158238 > mozilla-b2g:master

LGTM

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Kevin Grandon :kgrandon from comment #17)
> Comment on attachment 8599552 [details] [review]
> [gaia] KevinGrandon:reland_bug_1158238 > mozilla-b2g:master
> 
> Alive - wondering if you can take a look at this. It this may have a slight
> impact for your base module patch, and I can help rebase your patch if
> necessary. I assume that we may want to lazy load both browser.js and
> import.js. I think activity_handler needs to be there on startup though for
> the mozSetMessageHandler call?
> 
> Let me know if you have any questions, thanks!

It's ok - this one is compatible with my patch. What we need is have ActivityHandler to load + start import I think.

Comment 20

[Kevin Grandon :kgrandon]

Great, thanks Alive!

Comment 21

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#LEo1pnbeR9yybylF3KAI-w

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 22

[Kevin Grandon :kgrandon]

Seems like a Gip infra issue =/ Trying again.

Comment 23

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#Xhqva16VSIui0j4XyIZxUQ

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 24

[Kevin Grandon :kgrandon]

These python tests are killing me =(

Gaia-try is green though, so I think we should be ok here. Manually landing for now: https://github.com/mozilla-b2g/gaia/commit/bf5c2575cc0362556860238f8ca6953d67c2ed9c