Closed Bug 1121217 Opened 5 years ago Closed 4 years ago

Add a configure flag when building the b2g runtime for browser.html

Categories

(Firefox OS Graveyard :: Runtime, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: fabrice, Assigned: fabrice)

References

Details

(Whiteboard: graphene-larch)

Attachments

(2 files, 4 obsolete files)

No description provided.
Attached patch browser-html-config.patch (obsolete) — Splinter Review
Attachment #8548484 - Flags: review?(mh+mozilla)
Comment on attachment 8548484 [details] [diff] [review]
browser-html-config.patch

Review of attachment 8548484 [details] [diff] [review]:
-----------------------------------------------------------------

I know the context, but that doesn't tell me why it's needed. Especially, it's completely disconnected from anything else related. How is MOZ_BROWSER_HTML used? Where would --enable-browser-html be set?
Attachment #8548484 - Flags: review?(mh+mozilla) → feedback-
browser.html will reuse the b2g runtime, but we need a different behavior in some cases:
- different defaults prefs.
- custom code under b2g/ that is only for browser.html (because the way we startup is different).
- disabling MOZ_UA_OS_AGNOSTIC

and probably others that will come up with time.

We will turn on the flag when we set up the releng parts to create builds.
To me, it seems like you should have a --enable-application=browser-html or whatever codename you want to use, and if you need to build b2g stuff from there, well, you can add DIRS += ['../b2g'].
Attached patch browserhtml-product.patch (obsolete) — Splinter Review
After discussing on irc, we decided to create a new product, that builds like b2g for now but with its own confvars.sh and config/mozconfigs.

I can't push to try right now, will do as soon as try reopens.
Attachment #8548484 - Attachment is obsolete: true
Attachment #8549782 - Flags: review?(mh+mozilla)
Attached patch htmlrunner-product.patch (obsolete) — Splinter Review
Same patch except that I renamed the product to "htmlrunner".
Attachment #8549782 - Attachment is obsolete: true
Attachment #8549782 - Flags: review?(mh+mozilla)
Attachment #8550081 - Flags: review?(mh+mozilla)
Comment on attachment 8550081 [details] [diff] [review]
htmlrunner-product.patch

Review of attachment 8550081 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/confvars.sh
@@ +7,5 @@
>  
>  MOZ_APP_VERSION=38.0a1
>  MOZ_APP_UA_NAME=Firefox
>  
> +MOZ_B2G=1

It's already set on line 57.

::: htmlrunner/app.mozbuild
@@ +1,1 @@
> +# vim: set filetype=python:

htmlrunner sounds like webrunner and seems over-generic. browser.html had the benefit on making it clear that it was a browser.

::: htmlrunner/config/mozconfigs/common
@@ +12,5 @@
> +if test -d $topsrcdir/../gcc/bin; then
> +    HOST_CC="$topsrcdir/../gcc/bin/gcc"
> +    HOST_CXX="$topsrcdir/../gcc/bin/g++"
> +    ac_add_options --enable-stdcxx-compat
> +fi

This block is gonk specific. I doubt you'll need it.

::: htmlrunner/config/mozconfigs/linux32_gecko/debug
@@ +1,1 @@
> +. "$topsrcdir/b2g/config/mozconfigs/common"

seems to me this shouldn't be linux32_gecko, but someone from releng would tell for good. This also begs the question: do you really intend this to run on automation? Come to think of it, this does seem like a waste of resources, for something that is experimental. Wouldn't it be better for everyone if you could just download a b2g desktop build, edit a file, and run browser.html with it?

@@ +16,5 @@
> +
> +# Needed to enable breakpad in application.ini
> +export MOZILLA_OFFICIAL=1
> +
> +export MOZ_TELEMETRY_REPORTING=1

maybe not?

@@ +31,5 @@
> +ENABLE_MARIONETTE=1
> +export CXXFLAGS=-DMOZ_ENABLE_JS_DUMP
> +
> +# Include Firefox OS fonts.
> +MOZTTDIR=$topsrcdir/moztt

really?

::: htmlrunner/confvars.sh
@@ +7,5 @@
> +
> +MOZ_B2G=1
> +MOZ_HTMLRUNNER=1
> +
> +MOZ_APP_VERSION=38.0a1

Why not use $FIREFOX_VERSION? (b2g could do the same btw)

@@ +30,5 @@
> +NSS_DISABLE_DBM=1
> +MOZ_NO_EV_CERTS=1
> +MOZ_DISABLE_EXPORT_JS=1
> +
> +if test "$OS_TARGET" = "Android"; then

I doubt you'll ever build this.

@@ +39,5 @@
> +
> +# use custom widget for html:select
> +MOZ_USE_NATIVE_POPUP_WINDOWS=1
> +
> +if test "$LIBXUL_SDK"; then

no need to support LIBXUL_SDK, that option doesn't work anymore anyways.

@@ +47,5 @@
> +fi
> +
> +MOZ_MEDIA_NAVIGATOR=1
> +
> +MOZ_APP_ID={3c2e2abc-06d4-11e1-ac3b-374f68613e61}

seems to me you should use a different id.
Attachment #8550081 - Flags: review?(mh+mozilla) → feedback+
Closing for now as I think we finally won't need that once we land bug 1118138.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Reopening as tweaking the current b2g will really be ugly. htmlrunner patch coming.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Attached patch htmlrunner-product.patch (obsolete) — Splinter Review
Updated patch. I'm checking with releng to make builds happen.
Attachment #8550081 - Attachment is obsolete: true
Attachment #8555547 - Flags: review?(mh+mozilla)
Blocks: 1126650
Not sure if it's relevant for this patch or not, but do we want to keep /b2g/chrome code in htmlrunner? Maybe we want htmlrunner to provide its own shell code.
Attachment #8555547 - Flags: review?(mh+mozilla)
Attached patch graphene.patchSplinter Review
Alright, after quite some discussions, we decided to go with a simple flag (--enable-graphene) and use that in b2g/ to tweak whatever we need.

I added the mozconfigs according to what catlee agreed on to do builds.
Attachment #8555547 - Attachment is obsolete: true
Attachment #8559437 - Flags: review?(mh+mozilla)
Attachment #8559437 - Flags: review?(mh+mozilla)
Fabrice, in b2g/app/macbuild/Contents/Info.plist.in, b2g should become graphene. Using %APP_BINARY% should do it.
Attached patch fix plistSplinter Review
This is required to build a valid mac package.
Blocks: 1134106
Blocks: 1154182
No longer blocks: 1154182
Landed in mozilla-central via bug 1204965.
Status: REOPENED → RESOLVED
Closed: 5 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.