1345976 - [geckoview] Add bundle event-based GeckoView.loadUri()

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, enhancement)

Description

[Eugen Sawin [:esawin]]

Currently, GeckoView.loadUri is implemented via a native call to nsWindow::GeckoViewSupport::LoadUri, which defers it to the respective nsIBrowserDOMWindow::OpenURI override.

This implementation therefore depends on GeckoView being attached to a window, which introduces a race at startup.

With bug 1343613, we have queuing mechanics in the event dispatcher, which allows us to handle an early call to GeckoView.loadUri gracefully when it's based on bundle events.

Comment 1

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1345976-1.0-Add-GeckoView.loadUri-based-on-bundl.patch

Add basic but easily extendable bundle event-based loadUri(uri) and keep original loadUri(uri, flags) for Fennec compatibility for now.

Eventually, we should address load and reload flags in a consistent manner, at which point we may drop the original loadUri implementation.

Comment 2

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

Comment on attachment 8845598 [details] [diff] [review]
0001-Bug-1345976-1.0-Add-GeckoView.loadUri-based-on-bundl.patch

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

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoView.java
@@ +359,5 @@
> +    * Load the given URI.
> +    * @param uri The URI of the resource to load.
> +    */
> +    public void loadUri(String uri) {
> +        GeckoBundle msg = new GeckoBundle();

final GeckoBundle msg = new GeckoBundle(1)

::: mobile/android/modules/GeckoViewNavigation.jsm
@@ +43,4 @@
>        "GeckoView:GoBack",
>        "GeckoView:GoForward",
>        "GeckoView:Reload",
> +      "GeckoView:LoadUri",

Alphabetical order

@@ +70,4 @@
>        case "GeckoView:Reload":
>          this.browser.reload();
>          break;
> +      case "GeckoView:LoadUri":

Alphabetical order

Comment 3

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

Comment on attachment 8845598 [details] [diff] [review]
0001-Bug-1345976-1.0-Add-GeckoView.loadUri-based-on-bundl.patch

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

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoView.java
@@ +358,5 @@
>      /**
> +    * Load the given URI.
> +    * @param uri The URI of the resource to load.
> +    */
> +    public void loadUri(String uri) {

I think this should probably be android.net.Uri, even though we're going to serialize it to send to Gecko. We should at least have an override that uses it.

Comment 4

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1345976-1.1-Add-GeckoView.loadUri-based-on-bundl.patch

Addressed comments.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e02235c16d63
[1.1] Add GeckoView.loadUri based on bundle events. r=snorp,jchen

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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