Make KitkatCaptioningBridge a singleton

Currently each ContentViewCore creates a KitkatCaptioningBridge on K+ devices.
And all of them listens to the system CaptioningManager.
So whenever the system captioning changes, all ContentViewCores will have to handle it separately.
This change simplifies that process by making KitkatCaptioningBridge a singleton.
All ContentViewCores become listeners to that singleton.
Whenever system captioning changes, a single CaptioningChangeDelegate class will handle the change.
It will then inform all ContentViewCores that are alive.
If all ContentViewCores are GC'ed, KitkatCaptioningBridge will stop listen to system CaptioningManager.

BUG=457850

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

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

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

@@ -97,8 +97,9 @@ import java.util.Map.Entry;
  * being tied to the view system.
  */
 @JNINamespace("content")
-public class ContentViewCore
-        implements AccessibilityStateChangeListener, ScreenOrientationObserver {
+public class ContentViewCore implements
+        AccessibilityStateChangeListener, ScreenOrientationObserver,
+        SystemCaptioningBridge.SystemCaptioningBridgeListener {
 
     private static final String TAG = "ContentViewCore";
 
@@ -631,7 +632,7 @@ public class ContentViewCore
         mRenderCoordinates.setDeviceScaleFactor(deviceScaleFactor);
         mAccessibilityManager = (AccessibilityManager)
                 getContext().getSystemService(Context.ACCESSIBILITY_SERVICE);
-        mSystemCaptioningBridge = CaptioningBridgeFactory.create(this);
+        mSystemCaptioningBridge = CaptioningBridgeFactory.getSystemCaptioningBridge(mContext);
         mGestureStateListeners = new ObserverList<GestureStateListener>();
         mGestureStateListenersIterator = mGestureStateListeners.rewindableIterator();
 
@@ -1475,7 +1476,7 @@ public class ContentViewCore
         ScreenOrientationListener.getInstance().addObserver(this, mContext);
         GamepadList.onAttachedToWindow(mContext);
         mAccessibilityManager.addAccessibilityStateChangeListener(this);
-        mSystemCaptioningBridge.registerBridge();
+        mSystemCaptioningBridge.addListener(this);
     }
 
     /**
@@ -1498,7 +1499,7 @@ public class ContentViewCore
         // locking and app switching.
         setTextHandlesTemporarilyHidden(true);
         hidePopupsAndPreserveSelection();
-        mSystemCaptioningBridge.unregisterBridge();
+        mSystemCaptioningBridge.removeListener(this);
     }
 
     /**
@@ -2602,7 +2603,7 @@ public class ContentViewCore
     private void onRenderProcessChange() {
         attachImeAdapter();
         // Immediately sync closed caption settings to the new render process.
-        mSystemCaptioningBridge.syncToDelegate();
+        mSystemCaptioningBridge.syncToListener(this);
     }
 
     /**
@@ -2910,13 +2911,9 @@ public class ContentViewCore
         return null;
     }
 
-    /**
-     * Set closed captioning text track style settings.
-     *
-     * @param settings The TextTrackSettings object containing the new settings.
-     */
     @TargetApi(Build.VERSION_CODES.KITKAT)
-    public void setTextTrackSettings(TextTrackSettings settings) {
+    @Override
+    public void onSystemCaptioningChanged(TextTrackSettings settings) {
         if (mNativeContentViewCore == 0) return;
         nativeSetTextTrackSettings(mNativeContentViewCore, settings.getTextTrackBackgroundColor(),
                 settings.getTextTrackFontFamily(), settings.getTextTrackFontStyle(),

content/public/android/java/src/org/chromium/content/browser/accessibility/captioning/CaptioningBridgeFactory.java

@@ -4,10 +4,9 @@
 
 package org.chromium.content.browser.accessibility.captioning;
 
+import android.content.Context;
 import android.os.Build;
 
-import org.chromium.content.browser.ContentViewCore;
-
 /**
  * Returns the best captions bridge available.  If the API level is lower than KitKat, a no-op
  * bridge is returned since those systems didn't support this functionality.
@@ -16,12 +15,12 @@ public class CaptioningBridgeFactory {
     /**
      * Create and return the best SystemCaptioningBridge available.
      *
-     * @param contentViewCore the ContentViewCore to create the bridge with
+     * @param context Context associated with the activity.
      * @return the best SystemCaptioningBridge available.
      */
-    public static SystemCaptioningBridge create(ContentViewCore contentViewCore) {
+    public static SystemCaptioningBridge getSystemCaptioningBridge(Context context) {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
-            return new KitKatCaptioningBridge(contentViewCore);
+            return KitKatCaptioningBridge.getInstance(context);
         }
         // On older systems, return a CaptioningBridge that does nothing.
         return new EmptyCaptioningBridge();

content/public/android/java/src/org/chromium/content/browser/accessibility/captioning/CaptioningChangeDelegate.java

@@ -8,13 +8,15 @@ import android.graphics.Color;
 import android.graphics.Typeface;
 import android.view.accessibility.CaptioningManager.CaptionStyle;
 
+
 import org.chromium.base.VisibleForTesting;
-import org.chromium.content.browser.ContentViewCore;
+import org.chromium.content.browser.accessibility.captioning.SystemCaptioningBridge.SystemCaptioningBridgeListener;
 
-import java.lang.ref.WeakReference;
 import java.text.DecimalFormat;
 import java.text.DecimalFormatSymbols;
 import java.util.Locale;
+import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * API level agnostic delegate for getting updates about caption styles.
@@ -30,9 +32,6 @@ public class CaptioningChangeDelegate {
     @VisibleForTesting
     public static final String DEFAULT_CAPTIONING_PREF_VALUE = "";
 
-    // Using a weak reference avoids cycles that might prevent GC of WebView's WebContents.
-    private final WeakReference<ContentViewCore> mWeakContentViewCore;
-
     private boolean mTextTrackEnabled;
 
     private String mTextTrackBackgroundColor;
@@ -42,6 +41,10 @@ public class CaptioningChangeDelegate {
     private String mTextTrackTextColor;
     private String mTextTrackTextShadow;
     private String mTextTrackTextSize;
+    // Using weak references to avoid preventing listeners from getting GC'ed.
+    // TODO(qinmin): change this to a HashSet that supports weak references.
+    private final Map<SystemCaptioningBridgeListener, Boolean> mListeners =
+            new WeakHashMap<SystemCaptioningBridgeListener, Boolean>();
 
     /**
      * @see android.view.accessibility.CaptioningManager.CaptioningChangeListener#onEnabledChanged
@@ -55,7 +58,6 @@ public class CaptioningChangeDelegate {
      * @see android.view.accessibility.CaptioningManager.CaptioningChangeListener#onFontScaleChanged
      */
     public void onFontScaleChanged(float fontScale) {
-        if (mWeakContentViewCore.get() == null) return;
         mTextTrackTextSize = androidFontScaleToPercentage(fontScale);
         notifySettingsChanged();
     }
@@ -72,8 +74,6 @@ public class CaptioningChangeDelegate {
      * @see android.view.accessibility.CaptioningManager.CaptioningChangeListener#onUserStyleChanged
      */
     public void onUserStyleChanged(CaptioningStyle userStyle) {
-        if (mWeakContentViewCore.get() == null) return;
-
         mTextTrackTextColor = androidColorToCssColor(userStyle.getForegroundColor());
         mTextTrackBackgroundColor = androidColorToCssColor(userStyle.getBackgroundColor());
 
@@ -99,12 +99,8 @@ public class CaptioningChangeDelegate {
 
     /**
      * Construct a new CaptioningChangeDelegate object.
-     *
-     * @param contentViewCore the ContentViewCore to associate with
-     *        this delegate
      */
-    public CaptioningChangeDelegate(ContentViewCore contentViewCore) {
-        mWeakContentViewCore = new WeakReference<ContentViewCore>(contentViewCore);
+    public CaptioningChangeDelegate() {
     }
 
     /**
@@ -287,16 +283,52 @@ public class CaptioningChangeDelegate {
     }
 
     private void notifySettingsChanged() {
-        final ContentViewCore contentViewCore = mWeakContentViewCore.get();
-        if (contentViewCore == null) return;
+        for (SystemCaptioningBridgeListener listener : mListeners.keySet()) {
+            notifyListener(listener);
+        }
+    }
+
+    /**
+     * Notify a listener about the current text track settings.
+     *
+     * @param listener the listener to notify.
+     */
+    public void notifyListener(SystemCaptioningBridgeListener listener) {
         if (mTextTrackEnabled) {
             final TextTrackSettings settings = new TextTrackSettings(
                     mTextTrackBackgroundColor, mTextTrackFontFamily, mTextTrackFontStyle,
                     mTextTrackFontVariant, mTextTrackTextColor, mTextTrackTextShadow,
                     mTextTrackTextSize);
-            contentViewCore.setTextTrackSettings(settings);
+            listener.onSystemCaptioningChanged(settings);
         } else {
-            contentViewCore.setTextTrackSettings(new TextTrackSettings());
+            listener.onSystemCaptioningChanged(new TextTrackSettings());
         }
     }
+
+    /**
+     * Add a listener for changes with the system CaptioningManager.
+     *
+     * @param listener The SystemCaptioningBridgeListener object to add.
+     */
+    public void addListener(SystemCaptioningBridgeListener listener) {
+        mListeners.put(listener, null);
+    }
+
+    /**
+     * Remove a listener from system CaptionManager.
+     *
+     * @param listener The SystemCaptioningBridgeListener object to remove.
+     */
+    public void removeListener(SystemCaptioningBridgeListener listener) {
+        mListeners.remove(listener);
+    }
+
+    /**
+     * Return whether there are listeners associated with this class.
+     *
+     * @return true if there are at least one listener, or false otherwise.
+     */
+    public boolean hasActiveListener() {
+        return !mListeners.isEmpty();
+    }
 }

content/public/android/java/src/org/chromium/content/browser/accessibility/captioning/EmptyCaptioningBridge.java

@@ -10,20 +10,20 @@ package org.chromium.content.browser.accessibility.captioning;
  */
 public class EmptyCaptioningBridge implements SystemCaptioningBridge {
     /**
-     * A no-op implementation of the syncToDelegate function.
+     * A no-op implementation of the syncToListener function.
      */
     @Override
-    public void syncToDelegate() {}
+    public void syncToListener(SystemCaptioningBridge.SystemCaptioningBridgeListener listener) {}
 
     /**
-     * No-op implementation of registerBridge.
+     * No-op implementation of addListener.
      */
     @Override
-    public void registerBridge() {}
+    public void addListener(SystemCaptioningBridge.SystemCaptioningBridgeListener listener) {}
 
     /**
-     * A no-op implementation of the unregisterBridge function.
+     * A no-op implementation of the removeListener function.
      */
     @Override
-    public void unregisterBridge() {}
+    public void removeListener(SystemCaptioningBridge.SystemCaptioningBridgeListener listener) {}
 }

content/public/android/java/src/org/chromium/content/browser/accessibility/captioning/KitKatCaptioningBridge.java

@@ -9,8 +9,6 @@ import android.content.Context;
 import android.os.Build;
 import android.view.accessibility.CaptioningManager;
 
-import org.chromium.content.browser.ContentViewCore;
-
 import java.util.Locale;
 
 /**
@@ -24,6 +22,7 @@ public class KitKatCaptioningBridge implements SystemCaptioningBridge {
 
     private final CaptioningChangeDelegate mCaptioningChangeDelegate;
     private final CaptioningManager mCaptioningManager;
+    private static KitKatCaptioningBridge sKitKatCaptioningBridge;
 
     /**
      * Bridge listener to inform the mCaptioningChangeDelegate when the mCaptioningManager
@@ -53,23 +52,35 @@ public class KitKatCaptioningBridge implements SystemCaptioningBridge {
         }
     }
 
+    /**
+     * Return the singleton instance of the captioning bridge for Kitkat+
+     *
+     * @param context the Context to associate with this bridge.
+     * @return the singleton instance of KitKatCaptioningBridge.
+     */
+    public static KitKatCaptioningBridge getInstance(Context context) {
+        if (sKitKatCaptioningBridge == null) {
+            sKitKatCaptioningBridge = new KitKatCaptioningBridge(context);
+        }
+        return sKitKatCaptioningBridge;
+    }
+
     /**
      * Construct a new KitKat+ captioning bridge
      *
-     * @param contentViewCore the ContentViewCore to associate with this bridge.
+     * @param context the Context to associate with this bridge.
      */
-    public KitKatCaptioningBridge(ContentViewCore contenViewCore) {
-        mCaptioningChangeDelegate = new CaptioningChangeDelegate(contenViewCore);
-        mCaptioningManager = (CaptioningManager) contenViewCore.getContext()
-                                     .getApplicationContext()
-                                     .getSystemService(Context.CAPTIONING_SERVICE);
+    private KitKatCaptioningBridge(Context context) {
+        mCaptioningChangeDelegate = new CaptioningChangeDelegate();
+        mCaptioningManager =
+                (CaptioningManager) context.getApplicationContext().getSystemService(
+                        Context.CAPTIONING_SERVICE);
     }
 
     /**
      * Force-sync the current closed caption settings to the delegate
      */
-    @Override
-    public void syncToDelegate() {
+    private void syncToDelegate() {
         mCaptioningChangeDelegate.onEnabledChanged(mCaptioningManager.isEnabled());
         mCaptioningChangeDelegate.onFontScaleChanged(mCaptioningManager.getFontScale());
         mCaptioningChangeDelegate.onLocaleChanged(mCaptioningManager.getLocale());
@@ -77,21 +88,30 @@ public class KitKatCaptioningBridge implements SystemCaptioningBridge {
                 getCaptioningStyleFrom(mCaptioningManager.getUserStyle()));
     }
 
-    /**
-     * Register this bridge for event changes with the system CaptioningManager.
-     */
     @Override
-    public void registerBridge() {
-        mCaptioningManager.addCaptioningChangeListener(mCaptioningChangeListener);
-        syncToDelegate();
+    public void syncToListener(SystemCaptioningBridge.SystemCaptioningBridgeListener listener) {
+        if (!mCaptioningChangeDelegate.hasActiveListener()) {
+            syncToDelegate();
+        }
+        mCaptioningChangeDelegate.notifyListener(listener);
     }
 
-    /**
-     * De-register this bridge from the system captioning manager.
-     */
     @Override
-    public void unregisterBridge() {
-        mCaptioningManager.removeCaptioningChangeListener(mCaptioningChangeListener);
+    public void addListener(SystemCaptioningBridge.SystemCaptioningBridgeListener listener) {
+        if (!mCaptioningChangeDelegate.hasActiveListener()) {
+            mCaptioningManager.addCaptioningChangeListener(mCaptioningChangeListener);
+            syncToDelegate();
+        }
+        mCaptioningChangeDelegate.addListener(listener);
+        mCaptioningChangeDelegate.notifyListener(listener);
+    }
+
+    @Override
+    public void removeListener(SystemCaptioningBridge.SystemCaptioningBridgeListener listener) {
+        mCaptioningChangeDelegate.removeListener(listener);
+        if (!mCaptioningChangeDelegate.hasActiveListener()) {
+            mCaptioningManager.removeCaptioningChangeListener(mCaptioningChangeListener);
+        }
     }
 
     /**
@@ -99,7 +119,6 @@ public class KitKatCaptioningBridge implements SystemCaptioningBridge {
      *
      * @param userStyle the platform CaptionStyle
      * @return a Chromium CaptioningStyle
-     *
      */
     private CaptioningStyle getCaptioningStyleFrom(CaptioningManager.CaptionStyle userStyle) {
         return CaptioningStyle.createFrom(userStyle);

content/public/android/java/src/org/chromium/content/browser/accessibility/captioning/SystemCaptioningBridge.java

@@ -9,17 +9,35 @@ package org.chromium.content.browser.accessibility.captioning;
  */
 public interface SystemCaptioningBridge {
     /**
-     * Calls all of the delegate's methods with the system's current captioning settings.
+     * Interface for listening to changed from SystemCaptioningBridge.
      */
-    public void syncToDelegate();
+    public interface SystemCaptioningBridgeListener {
+        /**
+         * Called when system captioning settings change.
+         *
+         * @param settings The TextTrackSettings object containing the new settings.
+         */
+        public void onSystemCaptioningChanged(TextTrackSettings settings);
+    }
 
     /**
-     * Register this bridge for event changes with the system CaptioningManager.
+     * Sync the system's current captioning settings with the listener.
+     *
+     * @param listener The SystemCaptioningBridgeListener object to receive all settings.
      */
-    public void registerBridge();
+    public void syncToListener(SystemCaptioningBridgeListener listener);
 
     /**
-     * Unregister this bridge from system CaptionManager. Must be called to avoid leaks.
+     * Add a listener for changes with the system CaptioningManager.
+     *
+     * @param listener The SystemCaptioningBridgeListener object to add.
      */
-    public void unregisterBridge();
+    public void addListener(SystemCaptioningBridgeListener listener);
+
+    /**
+     * Remove a listener from system CaptionManager.
+     *
+     * @param listener The SystemCaptioningBridgeListener object to remove.
+     */
+    public void removeListener(SystemCaptioningBridgeListener listener);
 }