1281890 - [geckoview] Allow to set chromeURI from Java

Status: RESOLVED FIXED

(GeckoView :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

Right now, `widget/android/nsWindow.cpp` always takes Gecko's defaultChromeURI pref.  That's not very GeckoView friendly; this ticket tracks adding a parameter to allow loading a different chromeURI from GeckoView (or a GeckoView consumer).

The intention is that GV consumers will not use browser.xul, but instead geckoview.xul; and that they'll provide content URLs from their assets/ directories.  This lets them do things without modifying the omnijar and especially Gecko's prefs *before* Gecko startup.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1281890 - Allow setting chromeURI from Java.

The intention is that GeckoView consumers will not use browser.xul,
but instead geckoview.xul; and that they'll provide content URLs from
their assets/ directories.  This lets them do things without modifying
the omnijar and in particular without setting Gecko's prefs *before*
Gecko startup.

Review commit: https://reviewboard.mozilla.org/r/60424/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60424/

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,

<gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,
> 
> <gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />

Later, yes:  I removed the attrs.xml for GeckoView to ease separating the two parts of the code.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://reviewboard.mozilla.org/r/60424/#review59094

This is great, thanks.

::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:1790
(Diff revision 1)
> +         * XUL container <tt>chrome://browser/content/geckoview.xul</tt>.  See
> +         * <a href="https://developer.mozilla.org/en/docs/toolkit.defaultChromeURI">https://developer.mozilla.org/en/docs/toolkit.defaultChromeURI</a>
> +         *
> +         * @return URI or null.
> +         */
> +        String getDefaultChromeURI();

Thanks for documenting this, maybe it will start a trend.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8764731 [details]
Bug 1281890 - Allow setting chromeURI from Java.

https://reviewboard.mozilla.org/r/60424/#review59096

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Nick Alexander :nalexander from comment #3)
> (In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> > Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,
> > 
> > <gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />
> 
> Later, yes:  I removed the attrs.xml for GeckoView to ease separating the
> two parts of the code.

IMO we shouldn't be adding more stuff to GeckoInterface, which has become a big dumping ground for unrelated things. That's why I'd much prefer an attribute for setting the chrome URI.

Also, GeckoView had mGeckoEventListener and mNativeEventListener to prevent exposing the "handleMessage" methods as public methods of GeckoView. Only actual APIs of GeckoView should be public.

Lastly, for the most part GeckoView.xul doesn't need to send "Gecko:Ready". We should get rid of GeckoView's remaining dependency on the "Gecko:Ready" message soon.

Comment 7

[Nick Alexander :nalexander [he/him]]

(In reply to Jim Chen [:jchen] [:darchons] from comment #6)
> (In reply to Nick Alexander :nalexander from comment #3)
> > (In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> > > Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,
> > > 
> > > <gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />
> > 
> > Later, yes:  I removed the attrs.xml for GeckoView to ease separating the
> > two parts of the code.
> 
> IMO we shouldn't be adding more stuff to GeckoInterface, which has become a
> big dumping ground for unrelated things. That's why I'd much prefer an
> attribute for setting the chrome URI.

I agree, but I'm not willing to block GV progress on getting all the exposed APIs correct.  GV needs to fly free of Fennec, and pushing stuff into GI and cleaning up later is the fastest way I see to cutting those cords.  If you or somebody else is willing to split GI, please do, but it's going to be hard to figure out what the GV interface should look like whilst it's still bolted to the belly of Fennec.

> Also, GeckoView had mGeckoEventListener and mNativeEventListener to prevent
> exposing the "handleMessage" methods as public methods of GeckoView. Only
> actual APIs of GeckoView should be public.

Agree.

> Lastly, for the most part GeckoView.xul doesn't need to send "Gecko:Ready".
> We should get rid of GeckoView's remaining dependency on the "Gecko:Ready"
> message soon.

I agree, but I didn't know this when I started :)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f08130ec8dc1
Allow setting chromeURI from Java. r=snorp

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f08130ec8dc1