[GeckoView] Intent to deprecate and replace loadUri

Firefox Development
#1

Hi All,

As you might have noticed, the GeckoSession#loadUri API has grown in number of arguments, and we don’t really have a reason to believe that we won’t add more arguments in the future.

To make the API more ergonomic I’m proposing that we move the API to a builder-like API.

For example, a load call that today is written as


session.loadUri(
	"https://mywebsite.com/...",
	"http://my-referrer.com",
	LOAD_FLAGS_NONE,
	additionalHeaders);

would be written as


session.loader()
	.url("https://mywebsite.com/...")
	.referrer("http://my-referrer.com")
	.flags(LOAD_FLAGS_NONE)
	.additionalHeaders(headers)
	.load();

This API change will follow the usual deprecation period of 3 releases before being removed completely.

What follows is the docs for the new API, please let me know if you have any concerns or comments.

Main entry point for loading URIs into a {@link GeckoSession}.

The simplest use case is loading a URIs with no extra options, this can
be accomplished by specifying the URI in {@link #uri} and then calling
{@link #load}, e.g.


    session.loader().uri("http://mozilla.org").load();

This class can also be used to load data: URIs, either from
a byte[] array or a String using {@link
#data}, e.g.


    session.loader().data("the data:1234,5678", "text/plain").load();

This class also allows you to specify some extra data, e.g. you can set
a referrer using {@link #referrer} which can either be a {@link
GeckoSession} or a plain URL string. You can also specify some Load
Flags using {@link #flags}.

The class is structured as a Builder, so method calls can be easily
chained, e.g.


    session.loader()
         .url("http://mozilla.org")
         .referrer("http://my-referrer.com")
         .flags(...)
         .load();
#2

What makes the existing API problematic? Is it having to maintain all the different combinations in GeckoView under the hood?

From a Java point of view the builder makes sense. But as a Kotlin consumer I’d expect a single loadUri() method with default arguments, which is the more idiomatic way and what you’d usually see in Kotlin code instead of a builder.

But even for Java consumers, assuming that a plain URL load is what you need most of the time, wouldn’t it be awkward to have to write session.loader().uri("http://mozilla.org").load();?

#3

What makes the existing API problematic? Is it having to maintain all the different combinations in GeckoView under the hood?

That’s one problem, yeah. The other problem is that if you have two or more arguments of the same type (e.g. two String arguments) you can easily swap them by mistake and the code will still compile but it will have a hard-to-find bug in it.

From a Java point of view the builder makes sense. But as a Kotlin consumer I’d expect a single loadUri() method with default arguments , which is the more idiomatic way and what you’d usually see in Kotlin code instead of a builder.

Yeah unfortunately we don’t have default arguments (or really named arguments) in Java. In a way the builder pattern is basically emulating that.

But even for Java consumers, assuming that a plain URL load is what you need most of the time, wouldn’t it be awkward to have to write session.loader().uri("http://mozilla.org").load(); ?

Yeah good point, we’re probably gonna have a convenience loadUri(final String uri) for this common case. If you want to do anything more complicated you need to use the loader.

#4

Talking to :snorp on matrix we’re erring on the side of having a public Loader class that can be used to load URIs and maintain defaults. Something like this

session.load(new Loader.Builder()
    .uri("https://mozilla.org")
    .referrer("...")
    .flags(...)
    .build());

The advantage of this is that an app can store the defaults in the loader instance, e.g.

private final Loader.Builder mLoader = new Loader.Builder()
    .flags(...)
    .headerFilter(...);

And then call

session.load(mLoader.uri("https://mozilla.com").build());
#5

Would it make sense for convenience to still support session.load(url) (that internally can use the builder)? Not really sure whether AC would use it… wondering mostly about people coming from WebView.

#6

Yeah that’s what I said here :slight_smile: :

#7

Yeah unfortunately we don’t have default arguments (or really named arguments) in Java. In a way the builder pattern is basically emulating that.

I don’t want to derail this thread. But do we think there are still strong reasons to not go with Kotlin like Android Components, Fenix and the Android platform (Android’s Kotlin-first approach)? I wonder if it’s time to revisit this decision. Especially since GeckoView is strongly bound to Android and cannot run in a standalone Java environment.