Merge widget/src/android from birch back into mozilla-central




Widget: Android
6 years ago
6 years ago


(Reporter: dougt, Assigned: dougt)



Firefox Tracking Flags

(Not tracked)



(1 attachment, 2 obsolete attachments)



6 years ago
Created attachment 574016 [details] [diff] [review]
patch v.1

This is the merge from birch back to mozilla-central.  We want to support both the native client as well as the xul client.  Much of this has been reviewed previously, but it doesn't hurt to get another review.

The one todo is figuring out where to move the AndroidHistory stuff.  Right now The implementation of AndroidHistory is in the toolkit places directory -- what we need to do is move it and have an idl instead of directly linking from widget... I will do this in a followup.

the next todo is merge the new stuff back into embedding/android on mozilla.  When I do, i can uncomment jGetAccessibilityEnabled and the other new methods+properties.  Another followup.

This hasn't been tested on the native client yet -- only on the xul client.  We will test the native client when we begin bring it back over to mozilla-central.
Attachment #574016 - Flags: review?(blassey.bugs)
Comment on attachment 574016 [details] [diff] [review]
patch v.1

Review of attachment 574016 [details] [diff] [review]:
----------------------------------------------------------------- and should be landable right away. With those landing along with this AndroidBridge.cpp and AndroidJavaWrappers.cpp should be landable without commented or ifdef'd out code. 

If that is not the case, please use #ifdefs with sensible flags rather than comments or if (0)

Finally, the java compositor is a new feature, please add a ifdef for it

::: widget/src/android/AndroidBridge.cpp
@@ +166,5 @@
> +    jGetAccessibilityEnabled = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "getAccessibilityEnabled", "()Z");
> +    jHandleGeckoMessage = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "handleGeckoMessage", "(Ljava/lang/String;)Ljava/lang/String;");
> +    jCheckUriVisited = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "checkUriVisited", "(Ljava/lang/String;)V");
> +    jMarkUriVisited = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "markUriVisited", "(Ljava/lang/String;)V");
> +    */

use an #ifdef rather than comment this out. Also, use that #ifdef around the functions that call these and in the header.

::: widget/src/android/AndroidJNI.cpp
@@ +53,5 @@
>  #include "mozilla/Services.h"
>  #include "nsINetworkLinkService.h"
> +// Android history hasn't landed yet.
> +// #include "nsAndroidHistory.h"


@@ +204,5 @@
> +Java_org_mozilla_gecko_GeckoAppShell_notifyUriVisited(JNIEnv *jenv, jclass, jstring uri)
> +{
> +    // XXX - Android History hasn't landed on mozilla-central yet.
> +    //    nsAndroidHistory::NotifyURIVisited(nsJNIString(uri, jenv));


::: widget/src/android/AndroidJavaWrappers.cpp
@@ +167,5 @@
>      jRectField = getField("mRect", "Landroid/graphics/Rect;");
>      jCharactersField = getField("mCharacters", "Ljava/lang/String;");
> +    // This can be enabled when the embedding/android bits land on m-c
> +    //    jCharactersExtraField = getField("mCharactersExtra", "Ljava/lang/String;");

the patch to this file and the patch to should land together

::: widget/src/android/nsWindow.cpp
@@ +307,5 @@
> +{
> +    nsIntRect entireRect(0, 0, TILE_WIDTH, TILE_HEIGHT);
> +    AndroidGeckoEvent *event = new AndroidGeckoEvent(AndroidGeckoEvent::DRAW,
> +                                                     entireRect);
> +    nsAppShell::gAppShell->PostEvent(event);

we may need an ifdef/else here. The old code sent "new AndroidGeckoEvent(-1, -1, -1, -1);"

@@ +1150,5 @@
>          return;
>      }
>      AndroidBridge::AutoLocalJNIFrame jniFrame;
> +    if (0) { // java compositor

#ifdef JAVA_COMPOSITOR please
Attachment #574016 - Flags: review?(blassey.bugs) → review-

Comment 2

6 years ago
Created attachment 574073 [details] [diff] [review]
patch v.2

this has the embedding/android bits so that we don't have to comment out stuff in widget/
Attachment #574016 - Attachment is obsolete: true
Attachment #574073 - Flags: review?(blassey.bugs)

Comment 3

6 years ago

I don't want to have a #ifdef.  Instead, lets just define an interface, have it implemented by the native ui, and not the xul ui.  It will keep the widget code cleaner by not having #ifdefs.  We can keep the comments in the code, land this, and fix it up doubletime (e.g. days)


I don't want to have a #ifdef.  Instead, we can test to see if the class exists.  If it does, we can use it.  Otherwise, we will clear the exception and move on with life.  This is what the current patch does.

Comment 4

6 years ago

thinking about this more, we might need this because we have #ifdefs in places now to disable the default history store.  Either way, we can do this as a follow up.

Comment 5

6 years ago
that follow up is bug 702037.

Comment 6

6 years ago
Created attachment 574365 [details] [diff] [review]
patch v.2
Attachment #574073 - Attachment is obsolete: true
Attachment #574073 - Flags: review?(blassey.bugs)
Attachment #574365 - Flags: review?(blassey.bugs)
Attachment #574365 - Flags: review?(blassey.bugs) → review+


6 years ago
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.