Closed Bug 865207 Opened 12 years ago Closed 11 years ago

Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 24

People

(Reporter: ochameau, Assigned: ochameau)

Details

Attachments

(1 file, 7 obsolete files)

The Web apps actor shouldn't be specific to b2g so that it also works on Desktop and Fennec. If we want to build a new remote devtool to manage apps on all platform, we should start asap moving the actor to a place where we can load it from all platform and also start tuning it to make it work nicely everywhere. This bug is only about moving the actor, followup bugs will be required to make it work per platform.
Comment on attachment 743663 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ Patch on top of attachment 743634 [details] [diff] [review] from bug 844227. The xpcshell test already pass on Desktop, need to give it a try on Fennec.
Attachment #746531 - Attachment is obsolete: true
Attachment #743663 - Attachment is obsolete: true
Attachment #746533 - Attachment is obsolete: true
Attachment #748895 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Priority: -- → P2
Attachment #750070 - Attachment is obsolete: true
Comment on attachment 752705 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ After many random issues to make this patch have working xpcshell tests on desktop and b2g platform... the patch is now ready! So I'm moving the actor and its related test to devtools folder. I also registered this actor as other ones in DebuggerServer.addBrowserActors, but try to load it lazily (at least its JSM dependencies, but I think we should provide something in dbg-server to load the actor JS file on demand).
Attachment #752705 - Flags: review?(fabrice)
Attachment #752705 - Flags: review?(dcamp)
Attachment #752705 - Flags: review?(fabrice) → review+
Attachment #752705 - Attachment is obsolete: true
Attachment #752705 - Flags: review?(dcamp)
Comment on attachment 752880 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ Sorry fabrice, I realized lately that my patch wasn't applying to master due to recent devtool mass refactoring... Here is a new patch that work on last m-c changeset. The main difference is that I had to remove all #ifdef as the new devtools layout isn't supporting preprocessing anymore. So I mainly replaced various FileUtils.getDir(DIRRECTORY_NAME,...) with either DOMApplicationRegistry.getWebAppsBasePath() or DOMApplicationRegistry._getAppDir() And I also had to move the #ifdef MOZ_OFFICIAL from the actor to webapps.jsm. By the way, are you sure MOZ_OFFICIAL is the right env variable name to use? I haven't been able to see any usage of it in m-c, nor b2g. There is MOZ_OFFICIAL_BRANDING, but not just MOZ_OFFICIAL.
Attachment #752880 - Flags: review?(fabrice)
Attachment #752880 - Flags: review?(dcamp)
Comment on attachment 752880 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ Review of attachment 752880 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. ::: dom/apps/src/Webapps.jsm @@ +83,5 @@ > appsFile: null, > webapps: { }, > children: [ ], > allAppsLaunchable: false, > +#ifdef MOZ_OFFICIAL You're right about using MOZ_OFFICIAL_BRANDING instead. MOZ_OFFICIAL is from gaia... @@ +86,5 @@ > allAppsLaunchable: false, > +#ifdef MOZ_OFFICIAL > + get allowInstallingCertified() false, > +#else > + get allowInstallingCertified() true, I would name that allowSideloadingCertified
Attachment #752880 - Flags: review?(fabrice) → review+
Comment on attachment 752880 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ Review of attachment 752880 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/shell.js @@ +1012,5 @@ > if ("nsIProfiler" in Ci) { > DebuggerServer.addActors("resource://gre/modules/devtools/server/actors/profiler.js"); > } > DebuggerServer.addActors("resource://gre/modules/devtools/server/actors/styleeditor.js"); > DebuggerServer.addActors('chrome://browser/content/dbg-browser-actors.js'); Don't you still want webapps actors on b2g? Where is it added now?
Comment on attachment 752880 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ Review of attachment 752880 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/devtools/server/main.js @@ +183,5 @@ > if ("nsIProfiler" in Ci) > this.addActors("resource://gre/modules/devtools/server/actors/profiler.js"); > > this.addActors("resource://gre/modules/devtools/server/actors/styleeditor.js"); > + this.addActors("resource://gre/modules/devtools/server/actors/webapps.js"); dcamp: ^-- here! The actor is now registered on all platforms. It merely works on all platform, but needs some improvements on Android and Desktop to make it work nicely. Like ensuring registering intent on Android and start introducing webapps features in desktop (so far everything is made for webapprt, where apps are running in another process).
As of just recently, b2g doesn't reuse addBrowserActors. So you'll need to add that same line to shell.js too.
Comment on attachment 752880 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ ... but r+ with that change. Thanks!
Attachment #752880 - Flags: review?(dcamp) → review+
Attachment #752880 - Attachment is obsolete: true
Comment on attachment 755998 [details] [diff] [review] Move webapps actor from /b2g/chrome/content to /toolkit/devtools/apps/ r=fabrice r=dcamp Review comments addressed, rebase against last master, tested against b2g and fennec. Ready to land!
Attachment #755998 - Flags: review+
Keywords: checkin-needed
Assignee: nobody → poirot.alex
Flags: in-testsuite-
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: