Closed Bug 1291363 Opened 3 years ago Closed 3 years ago

[geckoview] Add new "geckoview" Gradle project

Categories

(GeckoView :: General, defect)

defect
Not set

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

The goal is to make a new "geckoview" Gradle project that compiles,
ships the minimal set of required resources (preferably none),
includes the Gecko libraries (libxul.so and friends), and can itself
be depended on by a "geckoview_example" project.  This Gradle project
will be, at first, a sibling to the existing "base" project.

The commit at
https://reviewboard.mozilla.org/r/60476/diff/1#0 shows how to do
this:

1) move geckoview sources out of mobile/android/base and into
   mobile/android/geckoview or similar.  It's possible to specify
   inclusion/exclusion to Gradle, but it's not easy; it's better to
   separate the two source trees, in preparation for Fennec itself to
   *compile* geckoview separately.  There's a shell script in that
   commit that should do this automatically.

2) update Fennec's moz.build to reference the new files (and Fennec's
   Gradle configuration).  See the commit.  Fennec's build will
   include the existing GeckoView files as usual.

3) add the new Gradle project.  Again, see the commit.

At this point, the new "geckoview" project will build a library (and,
at some point, an AAR file) that can be distributed independently of
Fennec.
Component: Embedding: APIs → GeckoView
Product: Core → Firefox for Android
This is the first part only.  It moves Experiments out of util (just
for convenience) -- I would have liked to have moved WhatsNewReceiver
out of notifications as well, but it's referenced in
AndroidManifest.xml.in and I don't want to break that.

The set of files in GeckoView is slightly larger than what we really
want, but it's close to correct.  Future tickets will get things
compiling and trim the set of files down to size.  Hopefully we can
move notifications/, permissions/, and restrictions/ back to Fennec.

This is not technically required, but it's best to bulk move the files
up front and patch later, rather than jump through hoops to
distinguish the set of files in Gradle (and moz.build).

Review commit: https://reviewboard.mozilla.org/r/68770/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68770/
Attachment #8777149 - Flags: review?(snorp)
Attachment #8777149 - Flags: review?(s.kaspari)
Comment on attachment 8777149 [details]
Bug 1291363 - Part 1: Move GeckoView sources into mobile/android/geckoview.

https://reviewboard.mozilla.org/r/68770/#review65906
Attachment #8777149 - Flags: review?(s.kaspari) → review+
Comment on attachment 8777149 [details]
Bug 1291363 - Part 1: Move GeckoView sources into mobile/android/geckoview.

Landed in Bug 1291811 with sebastian's r+.
Attachment #8777149 - Flags: review?(snorp)
Assignee: nobody → nalexander
sebastian, jchen: figure you two are in the best position to test building Fennec with this patch applied, and to test building geckoview{_exampe}, respectively.  The monkeying with the Gecko binaries is all to work around the |mach build| vs. |mach package| steps; see the comments in the new Gradle file.

chmanchester: you're here just to sanity check the TC changes.  Thanks!
Comment on attachment 8797212 [details]
Bug 1291363 - Make Gradle and Gradle Artifacts builds produce GeckoView AAR.

https://reviewboard.mozilla.org/r/82808/#review81542

I don't know these bits extremely well, but I sanity checked this is consistent with other stuff we're doing. If it works for you, it works for me.
Attachment #8797212 - Flags: review?(cmanchester) → review+
Comment on attachment 8797211 [details]
Bug 1291363 - Add geckoview and geckoview_example Gradle projects.

https://reviewboard.mozilla.org/r/82806/#review82216

The code LGTM and I can build and run Fennec and the sample. I needed to build fennec (app:assembleLocalDebug) once before I could open the project in AS again though. (nit: Some of the test classes seem to only contain dummy/failing code and reviewboard shows some trailing whitespaces/tabs in the gradle files).
Attachment #8797211 - Flags: review?(s.kaspari) → review+
Comment on attachment 8797211 [details]
Bug 1291363 - Add geckoview and geckoview_example Gradle projects.

https://reviewboard.mozilla.org/r/82806/#review81824

::: mobile/android/geckoview_example/src/androidTest/java/org/mozilla/geckoview_example/GeckoViewActivityTest.java:32
(Diff revision 1)
> +    @Test
> +    public void testA() throws InterruptedException {
> +        onView(withId(R.id.gecko_view))
> +                .check(matches(isDisplayed()));
> +
> +        Assert.assertEquals(1, 2);

Hm?

::: mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java:41
(Diff revision 1)
> +    protected void onStart() {
> +        super.onStart();
> +
> +        final GeckoProfile profile = GeckoProfile.get(getApplicationContext());
> +
> +        GeckoThread.init(profile, "chrome://browser/content/geckoview.xul", /* action */ null, /* debugging */ false);

Pass null for args. Specifying "chrome://browser/content/geckoview.xul" doesn't actually do anything.

::: mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java:50
(Diff revision 1)
> +    private class MyGeckoViewChrome implements GeckoView.ChromeDelegate {
> +        @Override
> +        public void onReady(GeckoView view) {
> +            Log.i(LOGTAG, "Gecko is ready");
> +            // Inject a script that adds some code to the content window
> +            mGeckoView.importScript("resource://android/assets/script.js");

There's no script.js?

::: mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java:105
(Diff revision 1)
> +            result.confirm();
> +        }
> +
> +        @Override
> +        public void onScriptMessage(GeckoView view, Bundle data, GeckoView.MessageResult result) {
> +        	Log.i(LOGTAG, "Got Script Message: " + data.toString());

These lines have a mixture of spaces and tabs.
Attachment #8797211 - Flags: review?(nchen) → review+
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/5d8f137ee52e
Add geckoview and geckoview_example Gradle projects. r=jchen,sebastian
https://hg.mozilla.org/integration/fx-team/rev/80d979342a04
Make Gradle build produce GeckoView AAR. r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/5d8f137ee52e
https://hg.mozilla.org/mozilla-central/rev/80d979342a04
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Depends on: 1308390
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 52 → mozilla52
You need to log in before you can comment on or make changes to this bug.