Throttle media decoding after excessive Android media server crashes

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

Index: content/public/android/java/src/org/chromium/content/browser/MediaThrottler.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/MediaThrottler.java b/content/public/android/java/src/org/chromium/content/browser/MediaThrottler.java
new file mode 100644
index 0000000000000000000000000000000000000000..e66a54596251e89b59eb9b6079c99350f4501938
--- /dev/null
+++ b/content/public/android/java/src/org/chromium/content/browser/MediaThrottler.java
@@ -0,0 +1,192 @@
+// Copyright 2015 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.content.browser;
+
+import android.content.Context;
+import android.media.MediaPlayer;
+import android.os.Handler;
+import android.os.Looper;
+import android.os.SystemClock;
+
+import org.chromium.base.Log;
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.base.annotations.JNINamespace;
+import org.chromium.content.R;
+
+/**
+ * Class for listening to Android MediaServer Crashes to throttle media decoding
+ * when needed.
+ */
+@JNINamespace("content")
+class MediaThrottler implements MediaPlayer.OnErrorListener {
+    private static final String TAG = "cr.MediaThrottler";

[Ted C, 2015/09/28 22:42:30]

there was an update on chromium dev:
[chromium-dev] [Android] Changes to the log tag format

but it should now be cr_ instead of cr.

[qinmin, 2015/09/29 23:16:23]

Done.

+    private static final long UNKNOWN_LAST_SERVER_CRASH_TIME = -1;
+
+    // Number of active decode requests.
+    private int mRequestCount;
+
+    // Application context.
+    private final Context mContext;
+
+    // Watch dog player. Used to listen to all media server crashes.
+    private MediaPlayer mPlayer;
+
+    // The last media server crash time since Chrome lauches.
+    private long mLastCrashTime = UNKNOWN_LAST_SERVER_CRASH_TIME;
+
+    // Server crash count since last reset() call.
+    private int mServerCrashCount = 0;

[Ted C, 2015/09/28 22:42:30]

0 is the default, no need to specify it.

[qinmin, 2015/09/29 23:16:23]

Done.

+
+    // Object for synchronized access to memeber variables.
+    private final Object mLock = new Object();
+
+    // Handler for posting delayed tasks.
+    private Handler mHandler;
+
+    // Intervals between media server crashes that are considered normal.
+    private static final long SERVER_CRASH_INTERVAL_THRESHOLD_IN_MILLIS = 60000;
+
+    // Delay to keep the watch dog player alive When there are no decoding
+    // requests. This is introduced to avoid recreating the watch dog over and
+    // over if a burst of small decoding requests arrive.
+    private static final int RELEASE_WATCH_DOG_PLAYER_DELAY_IN_MILLIS = 5000;
+
+    private final Runnable mDelayedReleaseRunnable = new Runnable() {
+        @Override
+        public void run() {
+            releaseWatchDogPlayer();
+        }
+    };
+
+    @CalledByNative
+    public static MediaThrottler create(Context context) {

[Ted C, 2015/09/28 22:42:30]

are these only called by native?  If so, they can all be private.

[qinmin, 2015/09/29 23:16:23]

Done.

+        return new MediaThrottler(context);
+    }
+
+    private MediaThrottler(Context context) {
+        mContext = context;
+        mHandler = new Handler(Looper.getMainLooper());
+    }
+
+    /*

[Ted C, 2015/09/28 22:42:30]

add another * to make it javadoc instead of a multiline comment (applies
elsewhere in this file)

[qinmin, 2015/09/29 23:16:23]

Done.

+     * Called to request the permission to decode media data.
+     *
+     * @return true if the request is permitted, or false otherwise.
+     */
+    @CalledByNative
+    public boolean requestToDecodeData() {
+        synchronized (mLock) {
+            long currentTime = SystemClock.elapsedRealtime();
+            if (mLastCrashTime != UNKNOWN_LAST_SERVER_CRASH_TIME
+                    && (currentTime - mLastCrashTime < SERVER_CRASH_INTERVAL_THRESHOLD_IN_MILLIS)
+                    && mServerCrashCount >= 4) {

[Ted C, 2015/09/28 22:42:30]

make 4 a constant as well for the maximum allowed

[qinmin, 2015/09/29 23:16:23]

Done.

+                Log.e(TAG, "Request to decode media data denied due to throttling.");
+                return false;
+            }
+            mRequestCount++;
+            if (mRequestCount == 1) {
+                mHandler.removeCallbacks(mDelayedReleaseRunnable);
+                startWatchDog();
+            }
+        }
+        return true;
+    }
+
+    /*
+     * Called to signal that a decode request has been completed.
+     */
+    @CalledByNative
+    public void onDecodeRequestFinished() {
+        synchronized (mLock) {
+            mRequestCount--;
+            if (mRequestCount == 0) {
+                // Don't release the watch dog immediately, there could be a
+                // number of small requests coming together.
+                prepareToStopWatchDog();
+            }
+        }
+    }
+
+    /*
+     * Starts the watch dog player to listen to media server crashes.
+     */
+    private void startWatchDog() {
+        new Runnable() {
+            @Override
+            public void run() {
+                synchronized (mLock) {
+                    if (mPlayer != null) return;
+                    mPlayer = MediaPlayer.create(mContext, R.raw.empty);
+                    if (mPlayer == null) {
+                        Log.e(TAG, "Unable to create watch dog player, treat it as server crash.");
+                        onMediaServerCrash();
+                    } else {
+                        mPlayer.setOnErrorListener(MediaThrottler.this);
+                    }
+                }
+            }
+        }.run();

[Ted C, 2015/09/28 22:42:30]

this doesn't actually post it to a different thread or anything, so there is no
benefit for it to be inside a runnable at all.  Should just have the
synchronized block directly inside the function

[qinmin, 2015/09/29 23:16:23]

oops, forget to post the runnable. Making it an AsyncTask instead, so that we
don't need a dedicated thread for this.

+    }
+
+    /*
+     * Posts a delayed task to stop the watch dog player.
+     */
+    private void prepareToStopWatchDog() {
+        mHandler.postDelayed(mDelayedReleaseRunnable, RELEASE_WATCH_DOG_PLAYER_DELAY_IN_MILLIS);
+    }
+
+    @Override
+    public boolean onError(MediaPlayer mp, int what, int extra) {
+        if (what == MediaPlayer.MEDIA_ERROR_SERVER_DIED) {
+            synchronized (mLock) {
+                onMediaServerCrash();
+            }
+        }
+        return true;
+    }
+
+    /*
+     * Called when media server crashes.
+     */
+    private void onMediaServerCrash() {
+        long currentTime = SystemClock.elapsedRealtime();
+        if (mLastCrashTime != UNKNOWN_LAST_SERVER_CRASH_TIME
+                && (currentTime - mLastCrashTime < SERVER_CRASH_INTERVAL_THRESHOLD_IN_MILLIS)) {
+            mServerCrashCount++;
+        } else {
+            mServerCrashCount = 1;
+        }
+        mLastCrashTime = currentTime;
+    }
+
+    /*
+     * Resets the MediaThrottler to its initial state so that subsequent requests
+     * will not be throttled.
+     */
+    @CalledByNative
+    public void reset() {
+        synchronized (mLock) {
+            mServerCrashCount = 0;
+            mLastCrashTime = -1;

[Ted C, 2015/09/28 22:42:30]

UNKNOWN_LAST_SERVER_CRASH_TIME instead of -1?

[qinmin, 2015/09/29 23:16:23]

Done.

+        }
+    }
+
+    /*
+     * Called to release the watch dog player.
+     */
+    private void releaseWatchDogPlayer() {
+        new Runnable() {

[Ted C, 2015/09/28 22:42:30]

same comment as above

[qinmin, 2015/09/29 23:16:23]

use an async task to do the release() call in background.

+            @Override
+            public void run() {
+                synchronized (mLock) {
+                    if (mRequestCount > 0) return;
+                    if (mPlayer == null) return;
+                    mPlayer.release();
+                    mPlayer = null;
+                }
+            }
+        }.run();
+    }
+}