1021481 - Handle app:// protocol for DEBUG=1 mode

Status: RESOLVED FIXED

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

Description

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

currently we use http:// for DEBUG=1 and app:// for device and b2g-desktop and we need to handle those two protocol shceme on build system like bug 1020167

we should register app:// protocol for DEBUG=1 mode to simplify build process.

Comment 1

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

I was wrong since bug 1020167 does not handle two protocol on build script, but we still use a lot of GAIA_SECHME (app:// or http:// for this variable) and GAIA_DOMAIN in $GAIA_DIR/build/*.

Comment 2

[Sherman Chen [:chens] (inactive)]

Attachment: WIP

This WIP will register app protocol for DEBUG=1 mode and redirect to http protocol, which serves app data by httpd.js.

But one problem comes into when I try to change scheme from |SCHEME=http://| to |SCHEME=app://|, it will get errors like: 

JavaScript error: http://system.gaiamobile.org:8080/shared/js/dump.js, line 24: settings is null
JavaScript error: http://system.gaiamobile.org:8080/js/applications.js, line 85: apps.mgmt is null

Comment 3

[Alexandre Poirot [:ochameau]]

I imagine the channel.redirectTo() call isn't transparent enough.
You may find some helpful tricks from the original app: protocol implementation, which was in JS in early releases
(and now is in C++):
http://mxr.mozilla.org/mozilla-b2g18/source/netwerk/protocol/app/AppProtocolHandler.js#53

Comment 4

[Sherman Chen [:chens] (inactive)]

Attachment: WIP

Rewrite app: protocol handler as comment 3 suggested, and also change browser-helper to avoid errors in comment 2. 

This patch works well on original homescreen, but if change to vertical homescreen , it will not have any icons on homescreen.

Comment 5

[Alexandre Poirot [:ochameau]]

May be that because of GAIA_PORT=8080?
Or something unexected now happens in post-manifest.js:manifestInterAppHostnames (which should be useless thanks to your patch)?

Comment 6

[Sherman Chen [:chens] (inactive)]

Tried on gaia master, still no icon on vertical homescreen,
gaia commit hash is: 646efa07944adfe96c28d6075f95be1037eb657c

Comment 7

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8441898 [details] [review]
WIP

Hi Yuren, how do you think about this patch? should we also take care GAIA_PORT in this one or file another bug to change it? 

As for vertical homescreen not showing any icon in DEBUG=1 mode, I've filed bug 1015865 to keep tracking it.

Comment 8

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8441898 [details] [review]
WIP

offline discussed with chens, we will keep GAIA_PORT for changing default http port and redirect app protocol to http server with GAIA_PORT.

Comment 9

[Sherman Chen [:chens] (inactive)]

Attachment: Pull Request

Support app protocol and keep GAIA_PORT

Comment 10

[Sherman Chen [:chens] (inactive)]

(In reply to Sherman Chen [:chens] from comment #7)
> As for vertical homescreen not showing any icon in DEBUG=1 mode, I've filed
> bug 1015865 to keep tracking it.

Correction, it should be bug 1027555.

Comment 11

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8443197 [details] [review]
Pull Request

I would like to give f+ for this pr and redirect review request to Alex since I'm new to nsIProtocolHandler.

and please file follow up bugs to remove domain or scheme from functions in utils such as getWebapp(), makeWebappsObject(), gaiaOriginURL(), gaiaManifestURL(), etc.

Comment 12

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

thanks chens correct me, I mean port or scheme, not domain.

Comment 13

[Sherman Chen [:chens] (inactive)]

(In reply to Yuren [:yurenju] from comment #11)
> and please file follow up bugs to remove domain or scheme from functions in
> utils such as getWebapp(), makeWebappsObject(), gaiaOriginURL(),
> gaiaManifestURL(), etc.

File bug 1028069 for follow up.

Comment 14

[Alexandre Poirot [:ochameau]]

Comment on attachment 8443197 [details] [review]
Pull Request

Looks good, thanks for this patch and the followup cleanup!

I dropped a note about how easily map this new app protocol to build_stage:
https://github.com/mozilla-b2g/gaia/pull/20654/files#discussion_r14067900

Comment 15

[Sherman Chen [:chens] (inactive)]

(In reply to Alexandre Poirot (:ochameau) from comment #14)
> Comment on attachment 8443197 [details] [review]
> Pull Request
> 
> Looks good, thanks for this patch and the followup cleanup!
> 
> I dropped a note about how easily map this new app protocol to build_stage:
> https://github.com/mozilla-b2g/gaia/pull/20654/files#discussion_r14067900

Getting rid of httpd sounds awesome! But we also need to take care of test-agent while mapping app protocol to build_stage. I would like to file another bug for that work, how do you think?

Comment 16

[Alexandre Poirot [:ochameau]]

(In reply to Sherman Chen [:chens] from comment #15)
> (In reply to Alexandre Poirot (:ochameau) from comment #14)
> > I dropped a note about how easily map this new app protocol to build_stage:
> > https://github.com/mozilla-b2g/gaia/pull/20654/files#discussion_r14067900
> 
> Getting rid of httpd sounds awesome! But we also need to take care of
> test-agent while mapping app protocol to build_stage. I would like to file
> another bug for that work, how do you think?

Sure, I didn't expected that it would work as-is.
We also need a filewatcher or something that would automagically refresh stage.
But we should start testing to see if test agent is the only thing left to fix.

Comment 17

[Sherman Chen [:chens] (inactive)]

(In reply to Alexandre Poirot (:ochameau) from comment #16)
> Sure, I didn't expected that it would work as-is.
> We also need a filewatcher or something that would automagically refresh
> stage.
> But we should start testing to see if test agent is the only thing left to
> fix.

File follow up bug for replace httpd: bug 1029423

Comment 18

[Sherman Chen [:chens] (inactive)]

Merged to master
https://github.com/mozilla-b2g/gaia/commit/7de7d836ac32a9803d503c2a776d4b08baf02e3f

Comment 19

[Sherman Chen [:chens] (inactive)]

Reopened because of regression in b2g-inbound.

Comment 20

[Carsten Book [:Tomcat]]

https://tbpl.mozilla.org/php/getParsedLog.php?id=42345470&tree=B2g-Inbound was the test failure/regression on b2g-i

Comment 21

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

revert commit
decffedef4c0c79f8b1faf6feceb9d51e3493269

Comment 22

[Sherman Chen [:chens] (inactive)]

Attachment: Pull Request

Comment 23

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8446321 [details] [review]
Pull Request

Change some manifest urls in app unit test, includes comms, settings, keyboard, and l10n in sharedtest.

Comment 24

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

could you also paste your tbpl link for gecko try server?

Comment 25

[Sherman Chen [:chens] (inactive)]

(In reply to Yuren [:yurenju] from comment #24)
> could you also paste your tbpl link for gecko try server?

Gaia-try:
https://tbpl.mozilla.org/?tree=Gaia-Try&rev=a2c027cf1c05040eb7fb51d70d799df96fb14fe0

Try: 
https://tbpl.mozilla.org/?tree=Try&rev=608980268a87

Comment 26

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8446321 [details] [review]
Pull Request

good to me, r=yurenju

Comment 27

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

and please separate to two commits next time to help us review more effectively.

Comment 28

[Sherman Chen [:chens] (inactive)]

Attachment: Pull Request - update before_script

This patch also update before_script for unit-tests-in-firefox 

Travis: 
https://travis-ci.org/mozilla-b2g/gaia/builds/28485594

Gaia-Try:
https://tbpl.mozilla.org/?tree=Gaia-Try&rev=57ccaf1ec701cf84287f5ca4da6c4e68c1a5704b 

Try: 
https://tbpl.mozilla.org/?tree=Try&rev=c80418e6ebdd

Comment 29

[Sherman Chen [:chens] (inactive)]

Merged to master
https://github.com/mozilla-b2g/gaia/commit/43fd8f8bff1f774cf5e9f9d8d3bbcfa2d4a30367