Closed Bug 746213 Opened 13 years ago Closed 13 years ago

Support mozApps.getSelf/getInstalled in desktop runtime to allow apps to access receipts

Categories

(Firefox Graveyard :: Webapp Runtime, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

(Whiteboard: [qa!])

Attachments

(1 file, 1 obsolete file)

Marketplace relies on getInstalled to determine if an app should be installable or just show "installed". The Marketplace app run natively currently would always show apps as installable because getSelf and getInstalled both run onsuccess with a null result.
Note, probably similar to bug 738832 but that one is about changing state (installing/uninstalling) where as this bug is just reading out state.
Blocks: 738832
OS: Mac OS X → All
Hardware: x86 → All
To be clear, getSelf() results in success with "null" while getInstalled results in success with "[]".
Depends on: 746771
Attached patch v1 (obsolete) (deleted) — Splinter Review
Not exactly how it worked before though in previous tryserver builds. But all this does is.. Import Webapps.jsm to initialize DOMApplicationRegistry, which handles Webapps:GetSelf, etc. messages.
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #616777 - Flags: review?(felipc)
Comment on attachment 616777 [details] [diff] [review] v1 cool, is this all that is needed for it to work? I'm giving the review here on the simple change (importing Webapps.jsm), but I don't know offhand if Webapps.jsm was prepared to be used in the runtime. It'd be great if Fabrice or someone could comment on that.
Attachment #616777 - Flags: review?(felipc) → review+
(In reply to Felipe Gomes (:felipe) from comment #4) > cool, is this all that is needed for it to work? At least on mac, it needs bug 746771 to correctly set registryDir. I noticed that the DOMRequest was being returned but no onsuccess/onerror. So I walked through the Firefox handler and saw that nothing was receiving the Webapps:GetSelf message which is usually handled by DOMApplicationRegistry. Just importing the file will make sure DOMApplicationRegistry is inited and ready to receive and send messages back to WebappsRegistry of Webapps.js.
Felipe: I modified Webapps.jsm to be usable by the runtime as part of the initial implementation of the launcher/shell in my fix for bug 725408: http://hg.mozilla.org/mozilla-central/diff/ef55c163a23a/dom/base/Webapps.jsm So it's ok to import it, and it's also necessary to do so, as Mardak notes. Ideally, we'd have a way to register modules to get loaded on startup, as we do components. But this is the way to do it in the meantime. The only suggestions I'd made for this patch are: 1. add a comment explaining why the module is being imported even though it isn't being used (otherwise someone might try to remove it later, thinking it cruft); 2. import it in the command-line handler, which is the initial entry point into the app and as close as you can get to "on startup" (although on the other hand it might be better to load it as lazily as possible to reduce impact on startup).
(In reply to Myk Melez [:myk] [@mykmelez] from comment #6) > 2. import it in the command-line handler Thinking a bit more about where to import Webapps.jsm.. Fabrice, any particular reason why Webapps.jsm shouldn't be imported by Webapp.js? The former, a js module, handles messages sent by the latter, a js component. But they don't work without the other. We run into this bug with the native runtime because the js module wasn't getting loaded.
(In reply to Edward Lee :Mardak from comment #7) > Fabrice, any particular reason why Webapps.jsm shouldn't be imported by > Webapp.js? The former, a js module, handles messages sent by the latter, a > js component. But they don't work without the other. Webapps.js is the content part of the API, and the .jsm is running in the parent process when using e10s. So you can't load the .jsm from the .js. The .jsm must indeed be loaded by the "chrome" to get and send messages. > We run into this bug with the native runtime because the js module wasn't > getting loaded. Expected. this is why we load the .jsm in nsBrowserGlue.js in fx desktop.
nsBrowserGlue.js is a component that loads on app-startup. It doesn't explicitly load Webapps.jsm, but it does define a lazy loader for webappsUI.jsm and then calls webappsUI.init() on final-ui-startup; webappsUI.jsm then loads Webapps.jsm. So Webapps.jsm gets loaded on final-ui-startup in Firefox. And perhaps we should do the same. But it seems sufficient for now to load it either when opening the first window or when handling the command-line (although it isn't clear when this happens relative to that notification). Aside: I'm concerned about the way Webapps.jsm loads the main registry file (<profile>/webapps/webapps.json) asynchronously. If the first thing a webapp does when it starts up is ask for its receipt, can it race the loading of that file? Seems like we should make sure it doesn't perhaps by queuing up getSelf etc. calls while that file is loading. But in any case that's a separate bug.
(In reply to Fabrice Desré [:fabrice] from comment #8) > The .jsm must indeed be loaded by the "chrome" to get and send messages. So then we should keep the import in content/webapp.js as opposed to in CommandLineHandler.js which would be running in a different context (?)
(In reply to Edward Lee :Mardak from comment #10) > (In reply to Fabrice Desré [:fabrice] from comment #8) > > The .jsm must indeed be loaded by the "chrome" to get and send messages. > So then we should keep the import in content/webapp.js as opposed to in > CommandLineHandler.js which would be running in a different context (?) By "chrome", Fabrice apparently means the chrome process, as opposed to the content process, where Webapps.js is executed when e10s is enabled (i.e. in Fennec). In the runtime, as in desktop Firefox, e10s is disabled, and there is only one process, so any script can load Webapps.jsm, even if it's in a different context from the browser/runtime chrome code.
Attached patch for checkin (deleted) — Splinter Review
Attachment #616777 - Attachment is obsolete: true
Component: Desktop Runtime → Web Apps
Product: Web Apps → Firefox
QA Contact: desktop-runtime → webapps
Version: unspecified → Trunk
Comment on attachment 616975 [details] [diff] [review] for checkin Could almost fall into a=desktop-only but webapprt/ isn't in the auto-approved list.
Attachment #616975 - Flags: approval-mozilla-central?
That looks fine to me, but you need a proper review before landing...
Is r+felipe not enough to land in webapprt/ ?
ha ok, he r+'d the previous one. As far as landing rules goes, that looks really safe for fennec indeed.
Returning to Web Apps product as we don't need separate mozilla-central approval: felipe: just got clearance from akeybl to land webapprt/ changes under the a=desktop-only rule
Component: Web Apps → Desktop Runtime
Flags: approval-mozilla-central?
Product: Firefox → Web Apps
QA Contact: webapps → desktop-runtime
Version: Trunk → unspecified
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Ed - Trying to figure out a good way to verify this. Shall I verify that I can integrate correctly with getSelf and getInstalled within the webrt for desktop with David's smoke tests for the mozapps API (the behavior on firefox should be the same as the webrt for desktop)?
I tested it by calling mozApps.install with the receipts object and checking it if it exists with mozApps.getSelf (in both Firefox and WebRT).
Blocks: 737571
Whiteboard: [qa+]
No longer blocks: 737571
Component: Desktop Runtime → Webapp Runtime
Product: Web Apps → Firefox
Verified on Win 7 64-bit. Created my own localhost test app to verify this. Also verified it through Ed's app as well.
Status: RESOLVED → VERIFIED
Whiteboard: [qa+] → [qa!]
No longer depends on: 746771
Depends on: 746771
Flags: in-moztrap?(jsmith)
QA Contact: desktop-runtime → jsmith
Flags: in-moztrap?(jsmith) → in-moztrap+
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: