Closed Bug 1158238 Opened 5 years ago Closed 5 years ago

[System] Add 'import-app' activity

Categories

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

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S11 (1may)

People

(Reporter: justindarc, Assigned: kgrandon)

References

Details

(Whiteboard: [systemsfe])

Attachments

(2 files, 2 obsolete files)

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.
Blocks: 1123846
Flags: needinfo?(kgrandon)
I will work on this.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Whiteboard: [systemsfe]
Test add-on for importing.
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!
Flags: needinfo?(kgrandon)
Attachment #8597376 - Flags: review?(jdarcangelo)
See Also: → 1158342
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.
See Also: 1158342
(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.
Target Milestone: --- → 2.2 S11 (1may)
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.
Attachment #8597376 - Flags: review?(jdarcangelo) → review-
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.
Attachment #8597376 - Flags: review- → review?(jdarcangelo)
Comment on attachment 8597376 [details] [review]
[gaia] KevinGrandon:bug_1158238_app_import_activity > mozilla-b2g:master

LGTM. Thanks for making those changes!
Attachment #8597376 - Flags: review?(jdarcangelo) → review+
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.
Attachment #8597376 - Flags: review+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
See Also: → 1159376
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.
Attachment #8598824 - Flags: review+
Keywords: checkin-needed
Depends on: 1159539
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8597376 - Attachment is obsolete: true
Attachment #8598824 - Attachment is obsolete: true
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!
Attachment #8599552 - Flags: review?(alive)
Comment on attachment 8599552 [details] [review]
[gaia] KevinGrandon:reland_bug_1158238 > mozilla-b2g:master

LGTM
Attachment #8599552 - Flags: review?(alive) → review+
(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.
Great, thanks Alive!
Keywords: checkin-needed
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.
Seems like a Gip infra issue =/ Trying again.
Keywords: checkin-needed
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.
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
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Depends on: 1183570
No longer depends on: 1183570
You need to log in before you can comment on or make changes to this bug.