[Android] Move weak ref for PopupWindowTouchHandleDrawable to native

The current weak reference wiring for PopupWindowTouchHandleDrawable is awkward
and fragile. Move the weak ref accounting to its native object counterpart,
using the JNI boundary as a consistent reference lifetime checkpoint.

BUG=528365

Review URL: https://codereview.chromium.org/1341013002

Cr-Commit-Position: refs/heads/master@{#348809}

content/browser/android/content_view_core_impl.cc

@@ -621,19 +621,6 @@ void ContentViewCoreImpl::OnSelectionEvent(ui::SelectionEventType event,
       selection_rect_pix.right(), selection_rect_pix.bottom());
 }
 
-scoped_ptr<ui::TouchHandleDrawable>
-ContentViewCoreImpl::CreatePopupTouchHandleDrawable() {
-  JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
-  if (obj.is_null()) {
-    NOTREACHED();
-    return scoped_ptr<ui::TouchHandleDrawable>();
-  }
-  return scoped_ptr<ui::TouchHandleDrawable>(new PopupTouchHandleDrawable(
-      Java_ContentViewCore_createPopupTouchHandleDrawable(env, obj.obj()),
-      dpi_scale_));
-}
-
 void ContentViewCoreImpl::ShowPastePopup(int x_dip, int y_dip) {
   RenderWidgetHostViewAndroid* view = GetRenderWidgetHostViewAndroid();
   if (!view)

content/browser/android/content_view_core_impl.h

@@ -263,7 +263,6 @@ class ContentViewCoreImpl : public ContentViewCore,
   void OnSelectionEvent(ui::SelectionEventType event,
                         const gfx::PointF& selection_anchor,
                         const gfx::RectF& selection_rect);
-  scoped_ptr<ui::TouchHandleDrawable> CreatePopupTouchHandleDrawable();
 
   void StartContentIntent(const GURL& content_url);
 

content/browser/android/popup_touch_handle_drawable.cc

@@ -4,59 +4,89 @@
 
 #include "content/browser/android/popup_touch_handle_drawable.h"
 
+#include "content/public/browser/android/content_view_core.h"
 #include "jni/PopupTouchHandleDrawable_jni.h"
 
 namespace content {
 
-PopupTouchHandleDrawable::PopupTouchHandleDrawable(
-    base::android::ScopedJavaLocalRef<jobject> drawable,
-    float dpi_scale)
-    : dpi_scale_(dpi_scale), drawable_(drawable) {
-  DCHECK(drawable.obj());
+// static
+scoped_ptr<PopupTouchHandleDrawable> PopupTouchHandleDrawable::Create(
+    ContentViewCore* content_view_core) {
+  DCHECK(content_view_core);
+  base::android::ScopedJavaLocalRef<jobject> content_view_core_obj =
+      content_view_core->GetJavaObject();
+  if (content_view_core_obj.is_null())
+    return nullptr;
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> drawable_obj(
+      Java_PopupTouchHandleDrawable_create(env, content_view_core_obj.obj()));
+  return scoped_ptr<PopupTouchHandleDrawable>(new PopupTouchHandleDrawable(
+      env, drawable_obj.obj(), content_view_core->GetDpiScale()));
+}
+
+PopupTouchHandleDrawable::PopupTouchHandleDrawable(JNIEnv* env,
+                                                   jobject obj,
+                                                   float dpi_scale)
+    : java_ref_(env, obj), dpi_scale_(dpi_scale) {
+  DCHECK(!java_ref_.is_empty());
 }
 
 PopupTouchHandleDrawable::~PopupTouchHandleDrawable() {
-  // Explicitly disabling ensures that any external references to the Java
-  // object are cleared, allowing it to be GC'ed in a timely fashion.
-  SetEnabled(false);
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
+  if (!obj.is_null())
+    Java_PopupTouchHandleDrawable_destroy(env, obj.obj());
 }
 
 void PopupTouchHandleDrawable::SetEnabled(bool enabled) {
   JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
+  if (obj.is_null())
+    return;
   if (enabled)
-    Java_PopupTouchHandleDrawable_show(env, drawable_.obj());
+    Java_PopupTouchHandleDrawable_show(env, obj.obj());
   else
-    Java_PopupTouchHandleDrawable_hide(env, drawable_.obj());
+    Java_PopupTouchHandleDrawable_hide(env, obj.obj());
 }
 
 void PopupTouchHandleDrawable::SetOrientation(
     ui::TouchHandleOrientation orientation) {
   JNIEnv* env = base::android::AttachCurrentThread();
-  jobject obj = drawable_.obj();
-  Java_PopupTouchHandleDrawable_setOrientation(env, obj,
-                                               static_cast<int>(orientation));
+  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
+  if (!obj.is_null()) {
+    Java_PopupTouchHandleDrawable_setOrientation(env, obj.obj(),
+                                                 static_cast<int>(orientation));
+  }
 }
 
 void PopupTouchHandleDrawable::SetAlpha(float alpha) {
   JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   bool visible = alpha > 0;
-  Java_PopupTouchHandleDrawable_setVisible(env, drawable_.obj(), visible);
+  if (!obj.is_null())
+    Java_PopupTouchHandleDrawable_setVisible(env, obj.obj(), visible);
 }
 
 void PopupTouchHandleDrawable::SetFocus(const gfx::PointF& position) {
   const gfx::PointF position_pix = gfx::ScalePoint(position, dpi_scale_);
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_PopupTouchHandleDrawable_setFocus(
-      env, drawable_.obj(), position_pix.x(), position_pix.y());
+  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
+  if (!obj.is_null()) {
+    Java_PopupTouchHandleDrawable_setFocus(env, obj.obj(), position_pix.x(),
+                                           position_pix.y());
+  }
 }
 
 gfx::RectF PopupTouchHandleDrawable::GetVisibleBounds() const {
   JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
+  if (obj.is_null())
+    return gfx::RectF();
   gfx::RectF unscaled_rect(
-      Java_PopupTouchHandleDrawable_getPositionX(env, drawable_.obj()),
-      Java_PopupTouchHandleDrawable_getPositionY(env, drawable_.obj()),
-      Java_PopupTouchHandleDrawable_getVisibleWidth(env, drawable_.obj()),
-      Java_PopupTouchHandleDrawable_getVisibleHeight(env, drawable_.obj()));
+      Java_PopupTouchHandleDrawable_getPositionX(env, obj.obj()),
+      Java_PopupTouchHandleDrawable_getPositionY(env, obj.obj()),
+      Java_PopupTouchHandleDrawable_getVisibleWidth(env, obj.obj()),
+      Java_PopupTouchHandleDrawable_getVisibleHeight(env, obj.obj()));
   return gfx::ScaleRect(unscaled_rect, 1.f / dpi_scale_);
 }
 

content/browser/android/popup_touch_handle_drawable.h

@@ -8,14 +8,17 @@
 #include "ui/touch_selection/touch_handle.h"
 
 #include "base/android/jni_android.h"
+#include "base/android/jni_weak_ref.h"
 
 namespace content {
 
+class ContentViewCore;
+
 // Touch handle drawable backed by an Android PopupWindow.
 class PopupTouchHandleDrawable : public ui::TouchHandleDrawable {
  public:
-  PopupTouchHandleDrawable(base::android::ScopedJavaLocalRef<jobject> drawable,
-                           float dpi_scale);
+  static scoped_ptr<PopupTouchHandleDrawable> Create(
+      ContentViewCore* content_view_core);
   ~PopupTouchHandleDrawable() override;
 
   // ui::TouchHandleDrawable implementation.
@@ -28,8 +31,11 @@ class PopupTouchHandleDrawable : public ui::TouchHandleDrawable {
   static bool RegisterPopupTouchHandleDrawable(JNIEnv* env);
 
  private:
+  PopupTouchHandleDrawable(JNIEnv* env, jobject obj, float dpi_scale);
+
+  JavaObjectWeakGlobalRef java_ref_;
+
   const float dpi_scale_;
-  base::android::ScopedJavaGlobalRef<jobject> drawable_;
 
   DISALLOW_COPY_AND_ASSIGN(PopupTouchHandleDrawable);
 };

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -40,6 +40,7 @@
 #include "content/browser/android/edge_effect_l.h"
 #include "content/browser/android/in_process/synchronous_compositor_impl.h"
 #include "content/browser/android/overscroll_controller_android.h"
+#include "content/browser/android/popup_touch_handle_drawable.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/gpu/browser_gpu_channel_host_factory.h"
 #include "content/browser/gpu/compositor_util.h"
@@ -1246,7 +1247,7 @@ scoped_ptr<ui::TouchHandleDrawable>
 RenderWidgetHostViewAndroid::CreateDrawable() {
   DCHECK(content_view_core_);
   if (!using_browser_compositor_)
-    return content_view_core_->CreatePopupTouchHandleDrawable();
+    return PopupTouchHandleDrawable::Create(content_view_core_);
 
   return scoped_ptr<ui::TouchHandleDrawable>(new CompositedTouchHandleDrawable(
       content_view_core_->GetLayer().get(),

content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

@@ -70,8 +70,6 @@ import org.chromium.content.browser.input.JoystickScrollProvider;
 import org.chromium.content.browser.input.LegacyPastePopupMenu;
 import org.chromium.content.browser.input.PastePopupMenu;
 import org.chromium.content.browser.input.PastePopupMenu.PastePopupMenuDelegate;
-import org.chromium.content.browser.input.PopupTouchHandleDrawable;
-import org.chromium.content.browser.input.PopupTouchHandleDrawable.PopupTouchHandleDrawableDelegate;
 import org.chromium.content.browser.input.SelectPopup;
 import org.chromium.content.browser.input.SelectPopupDialog;
 import org.chromium.content.browser.input.SelectPopupDropdown;
@@ -498,10 +496,6 @@ public class ContentViewCore implements
     private PastePopupMenu mPastePopupMenu;
     private boolean mWasPastePopupShowingOnInsertionDragStart;
 
-    private PopupTouchHandleDrawableDelegate mTouchHandleDelegate;
-
-    private PositionObserver mPositionObserver;
-
     // Size of the viewport in physical pixels as set from onSizeChanged.
     private int mViewportWidthPix;
     private int mViewportHeightPix;
@@ -900,7 +894,6 @@ public class ContentViewCore implements
             }
 
             mContainerView = containerView;
-            mPositionObserver = new ViewPositionObserver(mContainerView);
             mContainerView.setClickable(true);
             mViewAndroidDelegate.updateCurrentContainerView(mContainerView);
             for (ContainerViewObserver observer : mContainerViewObservers) {
@@ -1031,7 +1024,6 @@ public class ContentViewCore implements
         unregisterAccessibilityContentObserver();
         mGestureStateListeners.clear();
         ScreenOrientationListener.getInstance().removeObserver(this);
-        mPositionObserver.clearListener();
         mContainerViewObservers.clear();
         hidePopupsAndPreserveSelection();
         mPastePopupMenu = null;
@@ -1200,6 +1192,15 @@ public class ContentViewCore implements
         return onTouchEventImpl(event, isTouchHandleEvent);
     }
 
+    /**
+     * Called by PopupWindow-based touch handles.
+     * @param event the MotionEvent targeting the handle.
+     */
+    public boolean onTouchHandleEvent(MotionEvent event) {
+        final boolean isTouchHandleEvent = true;
+        return onTouchEventImpl(event, isTouchHandleEvent);
+    }
+
     private boolean onTouchEventImpl(MotionEvent event, boolean isTouchHandleEvent) {
         TraceEvent.begin("onTouchEvent");
         try {
@@ -1265,6 +1266,9 @@ public class ContentViewCore implements
                 || eventAction == MotionEvent.ACTION_POINTER_UP;
     }
 
+    /**
+     * @return Whether a scroll targeting web content is in progress.
+     */
     public boolean isScrollInProgress() {
         return mTouchScrollInProgress || mPotentiallyActiveFlingCount > 0;
     }
@@ -2570,36 +2574,6 @@ public class ContentViewCore implements
         return new MotionEventSynthesizer(this);
     }
 
-    @SuppressWarnings("unused")
-    @CalledByNative
-    private PopupTouchHandleDrawable createPopupTouchHandleDrawable() {
-        if (mTouchHandleDelegate == null) {
-            mTouchHandleDelegate = new PopupTouchHandleDrawableDelegate() {
-                @Override
-                public View getParent() {
-                    return getContainerView();
-                }
-
-                @Override
-                public PositionObserver getParentPositionObserver() {
-                    return mPositionObserver;
-                }
-
-                @Override
-                public boolean onTouchHandleEvent(MotionEvent event) {
-                    final boolean isTouchHandleEvent = true;
-                    return onTouchEventImpl(event, isTouchHandleEvent);
-                }
-
-                @Override
-                public boolean isScrollInProgress() {
-                    return ContentViewCore.this.isScrollInProgress();
-                }
-            };
-        }
-        return new PopupTouchHandleDrawable(mTouchHandleDelegate);
-    }
-
     /**
      * Initialize the view with an overscroll refresh handler.
      * @param handler The refresh handler.