The default bug view has changed. See this FAQ.

Avoid JNI for lock/unlock of ANPSurface

RESOLVED FIXED

Status

()

Firefox for Android
General
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: snorp, Assigned: snorp)

Tracking

unspecified
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Right now we use JNI and a ByteBuffer for allocating and passing around bitmap data when a plugin surface is locked and unlocked. This is inefficient and causes some memory management issues (sometimes crashes due to buffers being too large). It would be better to use a native solution similar to what the stock WebKit implementation does.
Created attachment 574794 [details] [diff] [review]
Use a native solution for locking/unlocking plugin surfaces
Created attachment 574795 [details] [diff] [review]
Use a native solution for locking/unlocking plugin surfaces
Attachment #574795 - Flags: review?(blassey.bugs)
Attachment #574794 - Attachment is obsolete: true
Blocks: 702643
Comment on attachment 574795 [details] [diff] [review]
Use a native solution for locking/unlocking plugin surfaces

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

I'll land this on birch, but we should figure out what to do about these struct definitions before merging to m-c. Please file a follow up bug

::: dom/plugins/base/android/ANPSurface.cpp
@@ +42,5 @@
>  
>  #define LOG(args...)  __android_log_print(ANDROID_LOG_INFO, "GeckoPlugins" , ## args)
>  #define ASSIGN(obj, name)   (obj)->name = anp_surface_##name
>  
> +// Copied from Android headers

I'm not sure how this should work license-wise. It might need to be a header in other-licenses that you #include

@@ +157,3 @@
>  
> +static bool anp_surface_lock(JNIEnv* env, jobject surfaceView, ANPBitmap* bitmap, ANPRectI* dirtyRect) {
> +  if (!bitmap || !surfaceView) {

no curly braces (here and elsewhere)

@@ +173,5 @@
> +  SurfaceInfo info;
> +  int err = gSurfaceFunctions.lock(surface, &info, NULL);
> +  if (err < 0) {
> +    return false;
> +  }

if (gSurfaceFunctions.lock(surface, &info, NULL) < 0)
   return false;
Comment on attachment 574795 [details] [diff] [review]
Use a native solution for locking/unlocking plugin surfaces

this crashes: 

I/GeckoAppShell( 5660): load "com.adobe.flashplayer.FlashPaintSurface" from "com.adobe.flashplayer" for "/datadata/com.adobe.flashplayer/lib/libflashplayer.so"
I/GeckoJNI( 5660): leaving void Java_org_mozilla_gecko_GeckoAppShell_executeNextRunnable(JNIEnv*, _jclass*)
I/GeckoAppShell( 5660): addPluginView:com.adobe.flashplayer.FlashPaintSurface@40618340 @ x:34.0 y:152.0 w:952.0 h:442.0
D/dalvikvm( 5660): GetFieldID: unable to find field Landroid/view/Surface;.mSurfacePointer:I
D/dalvikvm( 5660): GetFieldID: unable to find field Landroid/view/Surface;.mSurface:I
I/ActivityManager(  203): Process org.mozilla.fennec_blassey (pid 5660) has died.
Attachment #574795 - Flags: review?(blassey.bugs) → review-
Ok, looks like there is yet another field name I need to handle. What version of Android is this?
Created attachment 574889 [details] [diff] [review]
Use a native solution for locking/unlocking plugin surfaces

OK, this should work on 2.3+ now.
Attachment #574889 - Flags: review?(blassey.bugs)
Attachment #574795 - Attachment is obsolete: true
Attachment #574889 - Flags: review?(blassey.bugs) → review+
http://hg.mozilla.org/projects/birch/rev/bcb7930881c9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
You need to log in before you can comment on or make changes to this bug.