Observer Support for DistilledPagePrefs

components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefs.java

Index: components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefs.java
diff --git a/components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefs.java b/components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefs.java
index 3ec91baca8c91ad490fec48100bb72e9609e96e9..f7cba62ed17d6cea1194d2a40df7dbc6caae4fcf 100644
--- a/components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefs.java
+++ b/components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefs.java
@@ -4,7 +4,11 @@
 
 package org.chromium.components.dom_distiller.core;
 
+import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
+import org.chromium.base.NativeCall;
+
+import java.util.HashMap;
 
 /**
  * Wrapper for the dom_distiller::DistilledPagePrefs.
@@ -13,25 +17,85 @@ import org.chromium.base.JNINamespace;
 public final class DistilledPagePrefs {
 
     private final long mDistilledPagePrefsAndroid;
+    private HashMap<Observer, DistilledPagePrefsObserverWrapper> mObserverMap;
+
+    /**
+     * Observer interface for observing DistilledPagePrefs changes.
+     */
+    public interface Observer {
+        void onChangeTheme(Theme theme);
+    }
+
+    /**
+     * Wrapper for dom_distiller::android::DistilledPagePrefsObserverAndroid.
+     */
+    private static class DistilledPagePrefsObserverWrapper {
+        private final Observer mDistilledPagePrefsObserver;
+        private long mNativeDistilledPagePrefsObserverAndroidPtr;

[nyquist, 2014/07/23 17:30:46]

final

[sunangel, 2014/07/23 18:04:31]

Done.

+
+        DistilledPagePrefsObserverWrapper(Observer observer) {
+            mNativeDistilledPagePrefsObserverAndroidPtr = nativeInitObserverAndroid();
+            mDistilledPagePrefsObserver = observer;
+        }
+
+        @CalledByNative("DistilledPagePrefsObserverWrapper")
+        private void onChangeTheme(int theme) {
+            mDistilledPagePrefsObserver.onChangeTheme(Theme.getThemeForValue(theme));
+        }
+
+        private void destroy() {
+            nativeDestroyObserverAndroid(mNativeDistilledPagePrefsObserverAndroidPtr);
+        }
+
+        long getNativePtr() {

[nyquist, 2014/07/23 17:30:46]

This should have the same visibility as the constructor and destroy.

There are two ways of going about this, but I personally like what you did in
the TestObserver.

1) Make them all private.
2) Since the class is already private, it can only be accessed by the outer
class, so make the methods used by the outer class public to signify usage. So,
public constructor, destroy and getNativePtr.

Personally I prefer 2), but up to you.

[sunangel, 2014/07/23 18:04:31]

Done.

+            return mNativeDistilledPagePrefsObserverAndroidPtr;
+        }
+
+        @NativeCall("DistilledPagePrefsObserverWrapper")
+        private native long nativeInitObserverAndroid();
+
+        @NativeCall("DistilledPagePrefsObserverWrapper")
+        private native void nativeDestroyObserverAndroid(
+                long nativeDistilledPagePrefsObserverAndroid);
+    }
 
     DistilledPagePrefs(long distilledPagePrefsPtr) {
         mDistilledPagePrefsAndroid = nativeInit(distilledPagePrefsPtr);
+        mObserverMap = new HashMap<Observer, DistilledPagePrefsObserverWrapper>();
+    }
+
+    public void addObserver(Observer obs) {
+        DistilledPagePrefsObserverWrapper wrappedObserver =
+                new DistilledPagePrefsObserverWrapper(obs);
+        nativeAddObserver(mDistilledPagePrefsAndroid, wrappedObserver.getNativePtr());
+        mObserverMap.put(obs, wrappedObserver);
+    }
+
+    public void removeObserver(Observer obs) {
+        DistilledPagePrefsObserverWrapper wrappedObserver = mObserverMap.remove(obs);
+        if (wrappedObserver != null) {
+            nativeRemoveObserver(mDistilledPagePrefsAndroid, wrappedObserver.getNativePtr());
+            wrappedObserver.destroy();
+        }
     }
 
-    // TODO(sunangel): Add observer support from this Java class to native
-    // counterpart so UI can be updated across tabs.
     public void setTheme(Theme theme) {
         nativeSetTheme(mDistilledPagePrefsAndroid, theme.asNativeEnum());
     }
 
     public Theme getTheme() {
-        return Theme.getThemeForValue(
-            nativeGetTheme(mDistilledPagePrefsAndroid));
+        return Theme.getThemeForValue(nativeGetTheme(mDistilledPagePrefsAndroid));
     }
 
     private native long nativeInit(long distilledPagePrefPtr);
-    private native void nativeSetTheme(long nativeDistilledPagePrefsAndroid,
-        int theme);
-    private native int nativeGetTheme(
-        long nativeDistilledPagePrefsAndroid);
+
+    private native void nativeSetTheme(long nativeDistilledPagePrefsAndroid, int theme);
+
+    private native int nativeGetTheme(long nativeDistilledPagePrefsAndroid);
+
+    private native void nativeAddObserver(long nativeDistilledPagePrefsAndroid,
+            long nativeObserverPtr);
+
+    private native void nativeRemoveObserver(long nativeDistilledPagePrefsAndroid,
+            long nativeObserverPtr);
 }