DialogSurfaceManager implementation.

media/base/android/java/src/org/chromium/media/DialogSurfaceImpl.java

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.media;
+
+import android.content.Context;
+import android.os.Handler;
+import android.os.IBinder;
+
+import org.chromium.base.Log;
+
+/**
+ * Provide access to Dialog-based Surfaces.  It is unlikely that you want to
+ * use this class directly.  Instead, use the native wrappers for it in
+ * dialog_surface_holder.h .  If you must use these from Java, then it's likely
+ * that you should be using IDialogSurface instead.
+ *
+ * This class is thread-safe, since it gets calls from random binder threads and
+ * callbacks on the UI thread.  It interacts with DialogSurfaceCore to do the
+ * actual work of managing the DialogSurface.  It also hides all the threading
+ * by posting messages to a single thread for DialogSurfaceCore.
+ */
+class DialogSurfaceImpl
+        extends IDialogSurface.Stub implements DialogSurfaceWindowTokenProvider.Client {
+    private static final String TAG = "cr_media";
+
+    private final Handler mHandler;
+
+    // Note that we never do any work in response to a binder call with mLock
+    // held.  We always post elsewhere, which prevents reentrant calls from
+    // the client from deadlocking on mLock.

[boliu, 2017/01/04 01:48:44]

fwiw, synchronized blocks in java is re-entrant :)

[liberato (no reviews please), 2017/01/11 22:17:57]

not sure that they'd arrive on the same thread.  the client will thread hop away
from the binder thread, and binder will grab a new thread from the pool on our
side if it calls back in as a result.

[boliu, 2017/01/12 20:24:19]

Oh you meant re-entrant binder calls.

This comment doesn't seem to actually hold. release does a lot more than just
posting a task.

+    private final Object mLock = new Object();
+    private DialogSurfaceManagerImpl mOwner;
+    private DialogSurfaceCore mDialogCore;
+
+    /**
+     * Called from a random thread.
+     * Note that (pid, frameId) might be replaced by a token.
+     * @param rendererPid pid of owning renderer process
+     * @param renderFrameId render frame ID owned by Pid
+     * @param context Context that we use.
+     * @param owner Owning manager that we'll notify on release().
+     * @param handler handler for a thread with a looper.
+     * @param callback callback object to notify about state changes.
+     * @param x initial x position in chrome compositor (not screen) coords.
+     * @param y initial y position in chrome compositor (not screen) coords.
+     * @param width initial width.
+     * @param height initial height.
+     */
+    public DialogSurfaceImpl(int rendererPid, int renderFrameId, final Context context,
+            DialogSurfaceManagerImpl owner, Handler handler, final IDialogSurfaceCallback callback,
+            final int x, final int y, final int width, final int height) {
+        mOwner = owner;
+        mHandler = handler;
+
+        mDialogCore = new DialogSurfaceCore();
+
+        // Post init to the proper thread.
+        final DialogSurfaceCore dialogCore = mDialogCore;
+        Runnable r = new Runnable() {
+            @Override
+            public void run() {
+                dialogCore.initialize(context, callback, x, y, width, height);
+            }
+        };
+        mHandler.post(r);
+
+        // Register to get token updates.
+        mOwner.getWindowTokenProvider().registerClient(rendererPid, renderFrameId, this);
+    }
+
+    // Note that the native wrapper should call release() anyway on destruction.
+    protected void finalize() throws Throwable {

[boliu, 2017/01/04 01:48:44]

I gave a talk once about never using finalizers..

in this case, is asBinder().linkToDeath ok?

[liberato (no reviews please), 2017/01/11 22:17:57]

not sure.  the problem is that there's only the callback on the remote side, and
we have no guarantee that it ever dies.  i.e., a malicious client could drop our
IBinder stub, we'd become unreferenced, and nobody would call release.

methinks that we need this one, in case the client misbehaves.

[boliu, 2017/01/12 20:24:19]

According to a comment on an internal thread, an android engineer said
linkToDeath should work for this case. But can you test it? I tried looking at
the code, but it goes into the kernel, and I'm not brave enough to go further..

[liberato (no reviews please), 2017/02/03 21:28:32]

i'm not sure that i understand how linkToDeath will solve it.  we have nothing
to linkToDeath on, except the client callback.  but that can outlive |this| by
an arbitrarily long time, at the client's option.  it might even be re-used
across multiple DS instances.

how will that help?

actually, now that i think about it, we won't be finalized anyway. 
getWindowTokenProvider has us.

so maybe i'll drop the finalizer.

+        if (mDialogCore != null) {
+            Log.w(TAG, "Not released before finalization, releasing now");
+            release();
+        }
+        super.finalize();
+    }
+
+    /**
+     * Release the underlying surface, and generally clean up, in response to
+     * the client releasing the IDialogSurface.
+     */
+    @Override
+    public void release() {
+        synchronized (mLock) {
+            // If we've already been released, then do nothing.
+            if (mDialogCore == null) return;
+
+            // Unregister for token callbacks.
+            mOwner.getWindowTokenProvider().unregisterClient(this);
+
+            // Post a release on the proper thread to |mDialogCore|.
+            final DialogSurfaceCore dialogCore = mDialogCore;
+            Runnable r = new Runnable() {
+                @Override
+                public void run() {
+                    dialogCore.release();
+                }
+            };
+            mHandler.post(r);
+
+            // Notify our owner.  We don't bother to wait until the post
+            // completes, since it'll get cleaned up shortly anyway.
+            mOwner.notifyReleased();
+            mOwner = null;
+
+            mDialogCore = null;
+        }
+    }
+
+    @Override
+    public void scheduleLayoutSurface(final int x, final int y, final int width, final int height) {
+        // Random thread.
+        synchronized (mLock) {
+            if (mDialogCore == null) return;
+
+            final DialogSurfaceCore dialogCore = mDialogCore;
+            Runnable r = new Runnable() {

[boliu, 2017/01/04 01:48:44]

if this could happen every frame, can consider saving this runnable to avoid gc
churn

[liberato (no reviews please), 2017/01/11 22:17:57]

it can, in principle, if we ever try to use this for embedded content.  for full
screen, i'd expect one or two calls and that's it.

if we ever try embedded, then i think we'll need to improve this.

+                @Override
+                public void run() {
+                    dialogCore.layoutSurface(x, y, width, height);
+                }
+            };
+
+            mHandler.post(r);
+        }
+    }
+
+    @Override
+    public void onWindowToken(final IBinder token) {
+        synchronized (mLock) {
+            // Forward this change.
+            // Note that if we don't have a window token, then we could wait
+            // until we do, simply by skipping sending null if we haven't sent
+            // any non-null token yet.  If we're transitioning between windows,
+            // that might make the client's job easier, since it wouldn't have
+            // to guess when a new token is available.
+            final DialogSurfaceCore dialogCore = mDialogCore;
+            Runnable r = new Runnable() {
+                @Override
+                public void run() {
+                    dialogCore.onWindowToken(token);
+                }
+            };
+
+            mHandler.post(r);
+        }
+    }
+}