Closed Bug 701996 Opened 8 years ago Closed 8 years ago

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


(Core :: Widget: Android, defect)

Not set





(Reporter: dougt, Assigned: dougt)



(1 file, 2 obsolete files)

Attached patch patch v.1 (obsolete) — Splinter Review
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-
Attached patch patch v.2 (obsolete) — Splinter Review
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)

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.

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.
that follow up is bug 702037.
Attached patch patch v.2Splinter Review
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+
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.