Observer Support for DistilledPagePrefs

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

Index: components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefsObserverWrapper.java
diff --git a/components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefsObserverWrapper.java b/components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefsObserverWrapper.java
new file mode 100644
index 0000000000000000000000000000000000000000..97cf00ee46780ae8198a41ea01cb42c8f6716824
--- /dev/null
+++ b/components/dom_distiller/android/java/src/org/chromium/components/dom_distiller/core/DistilledPagePrefsObserverWrapper.java
@@ -0,0 +1,36 @@
+// Copyright 2014 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.components.dom_distiller.core;
+
+import org.chromium.base.CalledByNative;
+import org.chromium.base.JNINamespace;
+
+/**
+ * Wrapper for the dom_distiller::DistilledPagePrefsObserverWrapper.
+ */
+@JNINamespace("dom_distiller")

[nyquist, 2014/07/22 22:51:37]

this namespace should probably match that of DistilledPagePrefs

[sunangel, 2014/07/23 16:22:04]

Done.

+public class DistilledPagePrefsObserverWrapper {

[nyquist, 2014/07/22 22:51:37]

Could this be a static inner class of DistilledPagePrefs? At least make package
protected.

Also, I think in Java-land this could be called DistilledPagePrefsObserver if
you rename the native calss to DistilledPagePrefsObserverAndroid.

[sunangel, 2014/07/23 16:22:04]

Now static class.
Inner class now called ObserverWrapper. Not sure if this is what you wanted?
On 2014/07/22 22:51:37, nyquist wrote:

+
+    private final DistilledPagePrefs.Observer mDistilledPagePrefsObserver;
+    private long nativeDistilledPagePrefsObserverWrapperPtr;

[nyquist, 2014/07/22 22:51:37]

mNat...

[sunangel, 2014/07/23 16:22:03]

Done.

+
+    DistilledPagePrefsObserverWrapper(DistilledPagePrefs.Observer observer) {
+        nativeDistilledPagePrefsObserverWrapperPtr = nativeInit();
+        mDistilledPagePrefsObserver = observer;
+    }
+
+    @CalledByNative
+    private void onChangeTheme(int theme) {
+        mDistilledPagePrefsObserver.onChangeTheme(Theme.getThemeForValue(theme));
+    }
+
+    public long getNativePtr() {

[nyquist, 2014/07/22 22:51:37]

Could this be package protected? Seems like it's only used in
DistilledPagePrefs.

[sunangel, 2014/07/23 16:22:03]

Done.

+        return nativeDistilledPagePrefsObserverWrapperPtr;
+    }
+
+    private native long nativeInit();
+
+    private native void nativeDestroy(long nativeDistilledPagePrefsObserverWrapper);

[nyquist, 2014/07/22 22:51:37]

This class should probably have a destroy-method which calls nativeDestroy.
Otherwise you will be leaking the native object.

I argue you could call destroy() on this object once it has been removed from
the native list of observers.

[sunangel, 2014/07/23 16:22:04]

Done.

On 2014/07/22 22:51:37, nyquist wrote:

+}