aw: Lazy allocate AwGLFunctor

Right now the lifetime of AwGLFunctor (and that of associated objects)
roughly matches the lifetime of AwContents. This is not necessary, and
we only need to ensure it's alive when functor is in use. This better
matches the APIthat the vulkan functor will have. It also has the up
side of converting CleanupReference usage to explicit destroy, which
is always nice.

Bug: 901554
Change-Id: Ib1126d1ddfeacc7b8c689aeb51bb897cf839b41c
Reviewed-on: https://chromium-review.googlesource.com/c/1320749Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Chris Blume <cblume@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606512}

android_webview/glue/java/src/com/android/webview/chromium/DrawGLFunctor.java

@@ -22,20 +22,20 @@ class DrawGLFunctor implements AwContents.NativeDrawGLFunctor {
     private static final String TAG = DrawGLFunctor.class.getSimpleName();
 
     // Pointer to native side instance
-    private final DestroyRunnable mDestroyRunnable;
     private final WebViewDelegate mWebViewDelegate;
+    private long mNativeDrawGLFunctor;
 
     public DrawGLFunctor(long viewContext, WebViewDelegate webViewDelegate) {
-        mDestroyRunnable = new DestroyRunnable(nativeCreateGLFunctor(viewContext));
+        mNativeDrawGLFunctor = nativeCreateGLFunctor(viewContext);
         mWebViewDelegate = webViewDelegate;
     }
 
     @Override
     public void detach(View containerView) {
-        if (mDestroyRunnable.mNativeDrawGLFunctor == 0) {
+        if (mNativeDrawGLFunctor == 0) {
             throw new RuntimeException("detach on already destroyed DrawGLFunctor");
         }
-        mWebViewDelegate.detachDrawGlFunctor(containerView, mDestroyRunnable.mNativeDrawGLFunctor);
+        mWebViewDelegate.detachDrawGlFunctor(containerView, mNativeDrawGLFunctor);
     }
 
     private static final boolean sSupportFunctorReleasedCallback =
@@ -43,24 +43,23 @@ class DrawGLFunctor implements AwContents.NativeDrawGLFunctor {
 
     @Override
     public boolean requestDrawGL(Canvas canvas, Runnable releasedCallback) {
-        if (mDestroyRunnable.mNativeDrawGLFunctor == 0) {
+        if (mNativeDrawGLFunctor == 0) {
             throw new RuntimeException("requestDrawGL on already destroyed DrawGLFunctor");
         }
         assert canvas != null;
         if (sSupportFunctorReleasedCallback) {
             assert releasedCallback != null;
-            mWebViewDelegate.callDrawGlFunction(
-                    canvas, mDestroyRunnable.mNativeDrawGLFunctor, releasedCallback);
+            mWebViewDelegate.callDrawGlFunction(canvas, mNativeDrawGLFunctor, releasedCallback);
         } else {
             assert releasedCallback == null;
-            mWebViewDelegate.callDrawGlFunction(canvas, mDestroyRunnable.mNativeDrawGLFunctor);
+            mWebViewDelegate.callDrawGlFunction(canvas, mNativeDrawGLFunctor);
         }
         return true;
     }
 
     @Override
     public boolean requestInvokeGL(View containerView, boolean waitForCompletion) {
-        if (mDestroyRunnable.mNativeDrawGLFunctor == 0) {
+        if (mNativeDrawGLFunctor == 0) {
             throw new RuntimeException("requestInvokeGL on already destroyed DrawGLFunctor");
         }
         if (!sSupportFunctorReleasedCallback
@@ -69,7 +68,7 @@ class DrawGLFunctor implements AwContents.NativeDrawGLFunctor {
         }
 
         mWebViewDelegate.invokeDrawGlFunctor(
-                containerView, mDestroyRunnable.mNativeDrawGLFunctor, waitForCompletion);
+                containerView, mNativeDrawGLFunctor, waitForCompletion);
         return true;
     }
 
@@ -79,33 +78,16 @@ class DrawGLFunctor implements AwContents.NativeDrawGLFunctor {
     }
 
     @Override
-    public Runnable getDestroyRunnable() {
-        return mDestroyRunnable;
+    public void destroy() {
+        assert mNativeDrawGLFunctor != 0;
+        nativeDestroyGLFunctor(mNativeDrawGLFunctor);
+        mNativeDrawGLFunctor = 0;
     }
 
     public static void setChromiumAwDrawGLFunction(long functionPointer) {
         nativeSetChromiumAwDrawGLFunction(functionPointer);
     }
 
-    // Holds the core resources of the class, everything required to correctly cleanup.
-    // IMPORTANT: this class must not hold any reference back to the outer DrawGLFunctor
-    // instance, as that will defeat GC of that object.
-    private static final class DestroyRunnable implements Runnable {
-        private long mNativeDrawGLFunctor;
-        DestroyRunnable(long nativeDrawGLFunctor) {
-            mNativeDrawGLFunctor = nativeDrawGLFunctor;
-            assert mNativeDrawGLFunctor != 0;
-        }
-
-        // Called when the outer DrawGLFunctor instance has been GC'ed, i.e this is its finalizer.
-        @Override
-        public void run() {
-            assert mNativeDrawGLFunctor != 0;
-            nativeDestroyGLFunctor(mNativeDrawGLFunctor);
-            mNativeDrawGLFunctor = 0;
-        }
-    }
-
     private static native long nativeCreateGLFunctor(long viewContext);
     private static native void nativeDestroyGLFunctor(long functor);
     private static native void nativeSetChromiumAwDrawGLFunction(long functionPointer);

android_webview/java/src/org/chromium/android_webview/AwContents.java

@@ -264,12 +264,10 @@ public class AwContents implements SmartClipProvider {
         void detach(View containerView);
 
         /**
-         * Get a Runnable that is used to destroy the native portion of the functor. After the
-         * run method of this Runnable is called, no other methods should be called on the Java
-         * object.
+         * Destroy this functor instance and any native objects associated with it. No method is
+         * called after destroy.
          */
-        Runnable getDestroyRunnable();
-
+        void destroy();
     }
 
     /**
@@ -306,11 +304,8 @@ public class AwContents implements SmartClipProvider {
 
     private long mNativeAwContents;
     private final AwBrowserContext mBrowserContext;
-    // mContainerView and mCurrentFunctor form a pair that needs to stay in sync.
     private ViewGroup mContainerView;
-    private AwGLFunctor mCurrentFunctor;
-    private AwGLFunctor mInitialFunctor;
-    private AwGLFunctor mFullScreenFunctor; // Only non-null when in fullscreen mode.
+    private AwGLFunctor mDrawFunctor;
     private final Context mContext;
     private final int mAppTargetSdkVersion;
     private AwViewAndroidDelegate mViewAndroidDelegate;
@@ -744,9 +739,8 @@ public class AwContents implements SmartClipProvider {
             ThreadUtils.runOnUiThreadBlocking(() -> {
                 if (isDestroyedOrNoOperation(NO_WARN)) return;
                 if (level >= TRIM_MEMORY_MODERATE) {
-                    mInitialFunctor.deleteHardwareRenderer();
-                    if (mFullScreenFunctor != null) {
-                        mFullScreenFunctor.deleteHardwareRenderer();
+                    if (mDrawFunctor != null) {
+                        mDrawFunctor.deleteHardwareRenderer();
                     }
                 }
                 nativeTrimMemory(mNativeAwContents, level, visible);
@@ -828,8 +822,6 @@ public class AwContents implements SmartClipProvider {
             mAppTargetSdkVersion = mContext.getApplicationInfo().targetSdkVersion;
             mInternalAccessAdapter = internalAccessAdapter;
             mNativeDrawGLFunctorFactory = nativeDrawGLFunctorFactory;
-            mInitialFunctor = new AwGLFunctor(mNativeDrawGLFunctorFactory, mContainerView);
-            mCurrentFunctor = mInitialFunctor;
             mContentsClient = contentsClient;
             mContentsClient.getCallbackHelper().setCancelCallbackPoller(
                     () -> AwContents.this.isDestroyedOrNoOperation(NO_WARN));
@@ -938,13 +930,12 @@ public class AwContents implements SmartClipProvider {
         if (wasInitialContainerViewFocused) {
             fullScreenView.requestFocus();
         }
-        mFullScreenFunctor = new AwGLFunctor(mNativeDrawGLFunctorFactory, fullScreenView);
         mFullScreenTransitionsState.enterFullScreen(fullScreenView, wasInitialContainerViewFocused);
         mAwViewMethods = new NullAwViewMethods(this, mInternalAccessAdapter, mContainerView);
 
         // Associate this AwContents with the FullScreenView.
         setInternalAccessAdapter(fullScreenView.getInternalAccessAdapter());
-        setContainerView(fullScreenView, mFullScreenFunctor);
+        setContainerView(fullScreenView);
 
         return fullScreenView;
     }
@@ -985,15 +976,13 @@ public class AwContents implements SmartClipProvider {
 
         // Re-associate this AwContents with the WebView.
         setInternalAccessAdapter(mFullScreenTransitionsState.getInitialInternalAccessDelegate());
-        setContainerView(initialContainerView, mInitialFunctor);
+        setContainerView(initialContainerView);
 
         // Return focus to the WebView.
         if (mFullScreenTransitionsState.wasInitialContainerViewFocused()) {
             mContainerView.requestFocus();
         }
         mFullScreenTransitionsState.exitFullScreen();
-        // Drop AwContents last reference to this functor. AwGLFunctor is responsible for cleanup.
-        mFullScreenFunctor = null;
     }
 
     private void setInternalAccessAdapter(InternalAccessDelegate internalAccessAdapter) {
@@ -1001,15 +990,15 @@ public class AwContents implements SmartClipProvider {
         mViewEventSink.setAccessDelegate(mInternalAccessAdapter);
     }
 
-    private void setContainerView(ViewGroup newContainerView, AwGLFunctor currentFunctor) {
+    private void setContainerView(ViewGroup newContainerView) {
         // setWillNotDraw(false) is required since WebView draws it's own contents using it's
         // container view. If this is ever not the case we should remove this, as it removes
         // Android's gatherTransparentRegion optimization for the view.
         mContainerView = newContainerView;
-        mCurrentFunctor = currentFunctor;
-        updateNativeAwGLFunctor();
         mContainerView.setWillNotDraw(false);
 
+        assert mDrawFunctor == null;
+
         mViewAndroidDelegate.setContainerView(mContainerView);
         if (mAwPdfExporter != null) {
             mAwPdfExporter.setContainerView(mContainerView);
@@ -1118,9 +1107,18 @@ public class AwContents implements SmartClipProvider {
         }
     }
 
+    private void setFunctor(AwGLFunctor functor) {
+        if (mDrawFunctor == functor) return;
+        AwGLFunctor oldFunctor = mDrawFunctor;
+        mDrawFunctor = functor;
+        updateNativeAwGLFunctor();
+
+        if (oldFunctor != null) oldFunctor.releasedByContents();
+    }
+
     private void updateNativeAwGLFunctor() {
-        nativeSetAwGLFunctor(mNativeAwContents,
-                mCurrentFunctor != null ? mCurrentFunctor.getNativeAwGLFunctor() : 0);
+        nativeSetAwGLFunctor(
+                mNativeAwContents, mDrawFunctor != null ? mDrawFunctor.getNativeAwGLFunctor() : 0);
     }
 
     /* Common initialization routine for adopting a native AwContents instance into this
@@ -3279,6 +3277,7 @@ public class AwContents implements SmartClipProvider {
         // Only valid within software onDraw().
         private final Rect mClipBoundsTemporary = new Rect();
 
+        @SuppressLint("DrawAllocation") // For new AwGLFunctor.
         @Override
         public void onDraw(Canvas canvas) {
             if (isDestroyedOrNoOperation(NO_WARN)) {
@@ -3294,6 +3293,10 @@ public class AwContents implements SmartClipProvider {
                 return;
             }
 
+            if (canvas.isHardwareAccelerated() && mDrawFunctor == null) {
+                setFunctor(new AwGLFunctor(mNativeDrawGLFunctorFactory, mContainerView));
+            }
+
             mScrollOffsetManager.syncScrollOffsetFromOnDraw();
             int scrollX = mContainerView.getScrollX();
             int scrollY = mContainerView.getScrollY();
@@ -3324,7 +3327,7 @@ public class AwContents implements SmartClipProvider {
             }
             if (did_draw && canvas.isHardwareAccelerated()
                     && !ForceAuxiliaryBitmapRendering.sResult) {
-                did_draw = mCurrentFunctor.requestDrawGL(canvas);
+                did_draw = mDrawFunctor.requestDrawGL(canvas);
             }
             if (did_draw) {
                 int scrollXDiff = mContainerView.getScrollX() - scrollX;
@@ -3488,7 +3491,6 @@ public class AwContents implements SmartClipProvider {
                     mContainerView.getHeight());
             updateHardwareAcceleratedFeaturesToggle();
             postUpdateWebContentsVisibility();
-            mCurrentFunctor.onAttachedToWindow();
 
             updateDefaultLocale();
             mSettings.updateAcceptLanguages();
@@ -3512,7 +3514,7 @@ public class AwContents implements SmartClipProvider {
             mViewEventSink.onDetachedFromWindow();
             updateHardwareAcceleratedFeaturesToggle();
             postUpdateWebContentsVisibility();
-            mCurrentFunctor.onDetachedFromWindow();
+            setFunctor(null);
 
             if (mComponentCallbacks != null) {
                 mContext.unregisterComponentCallbacks(mComponentCallbacks);

android_webview/java/src/org/chromium/android_webview/AwGLFunctor.java

@@ -20,26 +20,7 @@ import org.chromium.base.annotations.JNINamespace;
  */
 @JNINamespace("android_webview")
 public class AwGLFunctor {
-    private static final class DestroyRunnable implements Runnable {
-        private final long mNativeAwGLFunctor;
-        private final Runnable mNativeDrawGLFunctorDestroyRunnable;
-
-        private DestroyRunnable(
-                long nativeAwGLFunctor, Runnable nativeDrawGLFunctorDestroyRunnable) {
-            mNativeAwGLFunctor = nativeAwGLFunctor;
-            mNativeDrawGLFunctorDestroyRunnable = nativeDrawGLFunctorDestroyRunnable;
-        }
-        @Override
-        public void run() {
-            mNativeDrawGLFunctorDestroyRunnable.run();
-            nativeDestroy(mNativeAwGLFunctor);
-        }
-    }
-
     private final long mNativeAwGLFunctor;
-    // Same gc-life time as this, but does not reference any members like |mContainerView|.
-    private final Object mLifetimeObject;
-    private final CleanupReference mCleanupReference;
     private final AwContents.NativeDrawGLFunctor mNativeDrawGLFunctor;
     private final ViewGroup mContainerView;
     private final Runnable mFunctorReleasedCallback;
@@ -49,23 +30,19 @@ public class AwGLFunctor {
     public AwGLFunctor(AwContents.NativeDrawGLFunctorFactory nativeDrawGLFunctorFactory,
             ViewGroup containerView) {
         mNativeAwGLFunctor = nativeCreate(this);
-        mNativeDrawGLFunctor = nativeDrawGLFunctorFactory.createFunctor(getAwDrawGLViewContext());
-        mLifetimeObject = new Object();
-        mCleanupReference = new CleanupReference(mLifetimeObject,
-                new DestroyRunnable(mNativeAwGLFunctor, mNativeDrawGLFunctor.getDestroyRunnable()));
+        mNativeDrawGLFunctor = nativeDrawGLFunctorFactory.createFunctor(
+                nativeGetAwDrawGLViewContext(mNativeAwGLFunctor));
         mContainerView = containerView;
         if (mNativeDrawGLFunctor.supportsDrawGLFunctorReleasedCallback()) {
             mFunctorReleasedCallback = () -> removeReference();
         } else {
             mFunctorReleasedCallback = null;
         }
-    }
-
-    public void onAttachedToWindow() {
         addReference();
     }
 
-    public void onDetachedFromWindow() {
+    public void releasedByContents() {
+        assert mRefCount > 0;
         removeReference();
     }
 
@@ -74,10 +51,12 @@ public class AwGLFunctor {
     }
 
     public long getNativeAwGLFunctor() {
+        assert mRefCount > 0;
         return mNativeAwGLFunctor;
     }
 
     public boolean requestDrawGL(Canvas canvas) {
+        assert mRefCount > 0;
         boolean success = mNativeDrawGLFunctor.requestDrawGL(canvas, mFunctorReleasedCallback);
         if (success && mFunctorReleasedCallback != null) {
             addReference();
@@ -90,11 +69,14 @@ public class AwGLFunctor {
     }
 
     private void removeReference() {
+        assert mRefCount > 0;
         if (--mRefCount == 0) {
             // When |mRefCount| decreases to zero, the functor is neither attached to a view, nor
             // referenced from the render tree, and so it is safe to delete the HardwareRenderer
             // instance to free up resources because the current state will not be drawn again.
-            deleteHardwareRenderer();
+            nativeDeleteHardwareRenderer(mNativeAwGLFunctor);
+            mNativeDrawGLFunctor.destroy();
+            nativeDestroy(mNativeAwGLFunctor);
         }
     }
 
@@ -110,13 +92,10 @@ public class AwGLFunctor {
     }
 
     public void deleteHardwareRenderer() {
+        assert mRefCount > 0;
         nativeDeleteHardwareRenderer(mNativeAwGLFunctor);
     }
 
-    public long getAwDrawGLViewContext() {
-        return nativeGetAwDrawGLViewContext(mNativeAwGLFunctor);
-    }
-
     /**
      * Intended for test code.
      * @return the number of native instances of this class.

android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java

@@ -443,22 +443,11 @@ public class AwTestContainerView extends FrameLayout {
         }
     }
 
-    private static final class NativeDrawGLFunctorDestroyRunnable implements Runnable {
-        public long mContext;
-        NativeDrawGLFunctorDestroyRunnable(long context) {
-            mContext = context;
-        }
-        @Override
-        public void run() {
-            mContext = 0;
-        }
-    }
-
     private class NativeDrawGLFunctor implements AwContents.NativeDrawGLFunctor {
-        private NativeDrawGLFunctorDestroyRunnable mDestroyRunnable;
+        private long mContext;
 
         NativeDrawGLFunctor(long context) {
-            mDestroyRunnable = new NativeDrawGLFunctorDestroyRunnable(context);
+            mContext = context;
         }
 
         @Override
@@ -470,14 +459,14 @@ public class AwTestContainerView extends FrameLayout {
         public boolean requestDrawGL(Canvas canvas, Runnable releasedRunnable) {
             assert releasedRunnable == null;
             if (!isBackedByHardwareView()) return false;
-            mHardwareView.requestRender(mDestroyRunnable.mContext, canvas, false);
+            mHardwareView.requestRender(mContext, canvas, false);
             return true;
         }
 
         @Override
         public boolean requestInvokeGL(View containerView, boolean waitForCompletion) {
             if (!isBackedByHardwareView()) return false;
-            mHardwareView.requestRender(mDestroyRunnable.mContext, null, waitForCompletion);
+            mHardwareView.requestRender(mContext, null, waitForCompletion);
             return true;
         }
 
@@ -487,8 +476,8 @@ public class AwTestContainerView extends FrameLayout {
         }
 
         @Override
-        public Runnable getDestroyRunnable() {
-            return mDestroyRunnable;
+        public void destroy() {
+            mContext = 0;
         }
     }