JavaBridge should use Annotation

Use an annotation to check whether or not a method should be exposed to JavaScript.  This is better
than relying on inheritence to determine whether or not to expose a method.

BUG=http://b/6910450


Review URL: https://chromiumcodereview.appspot.com/10830173

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150406 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/android/browser_jni_registrar.cc

@@ -16,6 +16,7 @@
 #include "content/browser/android/sandboxed_process_launcher.h"
 #include "content/browser/android/touch_point.h"
 #include "content/browser/geolocation/location_api_adapter_android.h"
+#include "content/browser/renderer_host/java/java_bound_object.h"
 #include "content/common/android/device_info.h"
 
 namespace {
@@ -30,6 +31,7 @@ base::android::RegistrationMethod kContentRegisteredMethods[] = {
   { "DeviceInfo", content::RegisterDeviceInfo },
   { "DownloadController",
     content::DownloadController::RegisterDownloadController },
+  { "JavaBoundObject", JavaBoundObject::RegisterJavaBoundObject },
   { "SandboxedProcessLauncher", content::RegisterSandboxedProcessLauncher },
   { "TouchPoint", content::RegisterTouchPoint },
   { "WebViewStatics", content::RegisterWebViewStatics },

content/browser/android/content_view_core_impl.cc

@@ -378,12 +378,12 @@ void ContentViewCoreImpl::AddJavascriptInterface(
     jobject /* obj */,
     jobject object,
     jstring name,
-    jboolean allow_inherited_methods) {
+    jboolean require_annotation) {
   ScopedJavaLocalRef<jobject> scoped_object(env, object);
   // JavaBoundObject creates the NPObject with a ref count of 1, and
   // JavaBridgeDispatcherHostManager takes its own ref.
   NPObject* bound_object = JavaBoundObject::Create(scoped_object,
-                                                   allow_inherited_methods);
+                                                   require_annotation);
   web_contents_->java_bridge_dispatcher_host_manager()->AddNamedObject(
       ConvertJavaStringToUTF16(env, name), bound_object);
   WebKit::WebBindings::releaseObject(bound_object);

content/browser/android/content_view_core_impl.h

@@ -117,7 +117,7 @@ class ContentViewCoreImpl : public ContentViewCore,
                               jobject obj,
                               jobject object,
                               jstring name,
-                              jboolean allow_inherited_methods);
+                              jboolean require_annotation);
   void RemoveJavascriptInterface(JNIEnv* env, jobject obj, jstring name);
 
   // --------------------------------------------------------------------------

content/browser/renderer_host/java/java_bound_object.cc

@@ -35,17 +35,19 @@ namespace {
 const char kJavaLangClass[] = "java/lang/Class";
 const char kJavaLangObject[] = "java/lang/Object";
 const char kJavaLangReflectMethod[] = "java/lang/reflect/Method";
+// TODO(dtrainor): Parameterize this so that WebView and Chrome for Android can
+// use different annotations.
+const char kJavaScriptInterfaceAnnotation[] =
+    "org/chromium/content/browser/JavascriptInterface";
 const char kGetClass[] = "getClass";
-const char kGetDeclaredMethods[] = "getDeclaredMethods";
 const char kGetMethods[] = "getMethods";
-const char kGetModifiers[] = "getModifiers";
-const char kReturningInteger[] = "()I";
+const char kIsAnnotationPresent[] = "isAnnotationPresent";
 const char kReturningJavaLangClass[] = "()Ljava/lang/Class;";
 const char kReturningJavaLangReflectMethodArray[] =
     "()[Ljava/lang/reflect/Method;";
+const char kTakesJavaLangClassReturningBoolean[] = "(Ljava/lang/Class;)Z";
 
-// This constant represents the value at java.lang.reflect.Modifier.PUBLIC.
-const int kJavaPublicModifier = 1;
+jclass g_safe_annotation_clazz = NULL;
 
 // Our special NPObject type.  We extend an NPObject with a pointer to a
 // JavaBoundObject.  We also add static methods for each of the NPObject
@@ -128,7 +130,7 @@ bool JavaNPObject::GetProperty(NPObject* np_object,
 // return value is simply converted to the corresponding NPAPI type.
 bool CallJNIMethod(jobject object, const JavaType& return_type, jmethodID id,
                    jvalue* parameters, NPVariant* result,
-                   bool allow_inherited_methods) {
+                   bool require_annotation) {
   JNIEnv* env = AttachCurrentThread();
   switch (return_type.type) {
     case JavaType::TypeBoolean:
@@ -210,7 +212,7 @@ bool CallJNIMethod(jobject object, const JavaType& return_type, jmethodID id,
         break;
       }
       OBJECT_TO_NPVARIANT(JavaBoundObject::Create(scoped_java_object,
-                                                  allow_inherited_methods),
+                                                  require_annotation),
                           *result);
       break;
     }
@@ -726,9 +728,8 @@ jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
 
 }  // namespace
 
-
 NPObject* JavaBoundObject::Create(const JavaRef<jobject>& object,
-                                  bool allow_inherited_methods) {
+                                  bool require_annotation) {
   // The first argument (a plugin's instance handle) is passed through to the
   // allocate function directly, and we don't use it, so it's ok to be 0.
   // The object is created with a ref count of one.
@@ -736,15 +737,15 @@ NPObject* JavaBoundObject::Create(const JavaRef<jobject>& object,
       &JavaNPObject::kNPClass));
   // The NPObject takes ownership of the JavaBoundObject.
   reinterpret_cast<JavaNPObject*>(np_object)->bound_object =
-      new JavaBoundObject(object, allow_inherited_methods);
+      new JavaBoundObject(object, require_annotation);
   return np_object;
 }
 
 JavaBoundObject::JavaBoundObject(const JavaRef<jobject>& object,
-                                 bool allow_inherited_methods)
+                                 bool require_annotation)
     : java_object_(object),
       are_methods_set_up_(false),
-      allow_inherited_methods_(allow_inherited_methods) {
+      require_annotation_(require_annotation) {
   // We don't do anything with our Java object when first created. We do it all
   // lazily when a method is first invoked.
 }
@@ -798,7 +799,7 @@ bool JavaBoundObject::Invoke(const std::string& name, const NPVariant* args,
   // Call
   bool ok = CallJNIMethod(java_object_.obj(), method->return_type(),
                           method->id(), &parameters[0], result,
-                          allow_inherited_methods_);
+                          require_annotation_);
 
   // Now that we're done with the jvalue, release any local references created
   // by CoerceJavaScriptValueToJavaValue().
@@ -810,6 +811,14 @@ bool JavaBoundObject::Invoke(const std::string& name, const NPVariant* args,
   return ok;
 }
 
+bool JavaBoundObject::RegisterJavaBoundObject(JNIEnv* env) {
+  g_safe_annotation_clazz = reinterpret_cast<jclass>(env->NewGlobalRef(
+      base::android::GetUnscopedClass(env, kJavaScriptInterfaceAnnotation)));
+  DCHECK(g_safe_annotation_clazz);
+
+  return true;
+}
+
 void JavaBoundObject::EnsureMethodsAreSetUp() const {
   if (are_methods_set_up_)
     return;
@@ -823,39 +832,36 @@ void JavaBoundObject::EnsureMethodsAreSetUp() const {
           kGetClass,
           kReturningJavaLangClass))));
 
-  const char* get_method = allow_inherited_methods_ ?
-      kGetMethods : kGetDeclaredMethods;
-
   ScopedJavaLocalRef<jobjectArray> methods(env, static_cast<jobjectArray>(
       env->CallObjectMethod(clazz.obj(), GetMethodIDFromClassName(
           env,
           kJavaLangClass,
-          get_method,
+          kGetMethods,
           kReturningJavaLangReflectMethodArray))));
 
   size_t num_methods = env->GetArrayLength(methods.obj());
-  if (num_methods <= 0)
-    return;
+  // Java objects always have public methods.
+  DCHECK(num_methods);
 
   for (size_t i = 0; i < num_methods; ++i) {
     ScopedJavaLocalRef<jobject> java_method(
         env,
         env->GetObjectArrayElement(methods.obj(), i));
 
-    bool is_method_allowed = true;
-    if (!allow_inherited_methods_) {
-      jint modifiers = env->CallIntMethod(java_method.obj(),
-                                          GetMethodIDFromClassName(
-                                              env,
-                                              kJavaLangReflectMethod,
-                                              kGetModifiers,
-                                              kReturningInteger));
-      is_method_allowed &= (modifiers & kJavaPublicModifier);
+    if (require_annotation_) {
+      jboolean safe = env->CallBooleanMethod(java_method.obj(),
+          GetMethodIDFromClassName(
+              env,
+              kJavaLangReflectMethod,
+              kIsAnnotationPresent,
+              kTakesJavaLangClassReturningBoolean),
+          g_safe_annotation_clazz);
+
+      if (!safe)
+        continue;
     }
 
-    if (is_method_allowed) {
-      JavaMethod* method = new JavaMethod(java_method);
-      methods_.insert(std::make_pair(method->name(), method));
-    }
+    JavaMethod* method = new JavaMethod(java_method);
+    methods_.insert(std::make_pair(method->name(), method));
   }
 }

content/browser/renderer_host/java/java_bound_object.h

@@ -23,13 +23,14 @@
 // created and destroyed on different threads.
 class JavaBoundObject {
  public:
-  // Takes a Java object and creates a JavaBoundObject around it. Also takes
-  // a boolean that determines whether or not inherited methods are allowed
-  // to be called as well.  This property propagates to all Objects that get
-  // implicitly exposed as return values as well. Returns an NPObject with
-  // a ref count of one which owns the JavaBoundObject.
+  // Takes a Java object and creates a JavaBoundObject around it. The
+  // |require_annotation| flag specifies whether or not only methods with the
+  // JavascriptInterface annotation are exposed to JavaScript.  This property
+  // propagates to all Objects that get implicitly exposed as return values as
+  // well. Returns an NPObject with a ref count of one which owns the
+  // JavaBoundObject.
   static NPObject* Create(const base::android::JavaRef<jobject>& object,
-                          bool allow_inherited_methods);
+                          bool require_annotation);
 
   virtual ~JavaBoundObject();
 
@@ -43,9 +44,11 @@ class JavaBoundObject {
   bool Invoke(const std::string& name, const NPVariant* args, size_t arg_count,
               NPVariant* result);
 
+  static bool RegisterJavaBoundObject(JNIEnv* env);
+
  private:
   explicit JavaBoundObject(const base::android::JavaRef<jobject>& object,
-                           bool allow_inherited_methods);
+                           bool require_annotation);
 
   void EnsureMethodsAreSetUp() const;
 
@@ -60,7 +63,7 @@ class JavaBoundObject {
   mutable JavaMethodMap methods_;
   mutable bool are_methods_set_up_;
 
-  bool allow_inherited_methods_;
+  const bool require_annotation_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(JavaBoundObject);
 };

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

@@ -1030,13 +1030,12 @@ public class ContentViewCore implements MotionEventDelegate {
      * @param object The Java object to inject into the ContentViewCore's
      *               JavaScript context. Null values are ignored.
      * @param name The name used to expose the instance in JavaScript.
-     * @param allowInheritedMethods Whether or not inherited methods may be
-     *                              called from JavaScript.
+     * @param requireAnnotation Restrict exposed methods to ones with the
+     *                          {@link JavascriptInterface} annotation.
      */
-    public void addJavascriptInterface(Object object, String name, boolean allowInheritedMethods) {
+    public void addJavascriptInterface(Object object, String name, boolean requireAnnotation) {
         if (mNativeContentViewCore != 0 && object != null) {
-            nativeAddJavascriptInterface(mNativeContentViewCore, object, name,
-                    allowInheritedMethods);
+            nativeAddJavascriptInterface(mNativeContentViewCore, object, name, requireAnnotation);
         }
     }
 
@@ -1200,7 +1199,7 @@ public class ContentViewCore implements MotionEventDelegate {
     private native int nativeEvaluateJavaScript(String script);
 
     private native void nativeAddJavascriptInterface(int nativeContentViewCoreImpl, Object object,
-                                                     String name, boolean allowInheritedMethods);
+                                                     String name, boolean requireAnnotation);
 
     private native void nativeRemoveJavascriptInterface(int nativeContentViewCoreImpl, String name);
 }

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

+// Copyright (c) 2012 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 java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Marks a method as being able to be exposed to JavaScript.  This is used for safety purposes so
+ * that only explicitly marked methods get exposed instead of every method in a class.
+ * @see {@link ContentViewCore#addJavascriptInterface(Class, String, boolean)}
+ */
+@SuppressWarnings("javadoc")
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.METHOD})
+public @interface JavascriptInterface {
+}
\ No newline at end of file

content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java

@@ -18,6 +18,7 @@ import android.view.accessibility.AccessibilityNodeInfo;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.utils.URLEncodedUtils;
 import org.chromium.content.browser.ContentViewCore;
+import org.chromium.content.browser.JavascriptInterface;
 import org.json.JSONException;
 import org.json.JSONObject;
 
@@ -239,13 +240,13 @@ public class AccessibilityInjector {
             if (mTextToSpeech == null) {
                 mTextToSpeech = new TextToSpeechWrapper(context);
                 mContentViewCore.addJavascriptInterface(mTextToSpeech,
-                        ALIAS_ACCESSIBILITY_JS_INTERFACE, false);
+                        ALIAS_ACCESSIBILITY_JS_INTERFACE, true);
             }
 
             if (mVibrator == null) {
                 mVibrator = new VibratorWrapper(context);
                 mContentViewCore.addJavascriptInterface(mVibrator,
-                        ALIAS_ACCESSIBILITY_JS_INTERFACE_2, false);
+                        ALIAS_ACCESSIBILITY_JS_INTERFACE_2, true);
             }
         }
     }
@@ -314,17 +315,20 @@ public class AccessibilityInjector {
             mVibrator = (Vibrator) context.getSystemService(Context.VIBRATOR_SERVICE);
         }
 
+        @JavascriptInterface
         @SuppressWarnings("unused")
         public boolean hasVibrator() {
             return mVibrator.hasVibrator();
         }
 
+        @JavascriptInterface
         @SuppressWarnings("unused")
         public void vibrate(long milliseconds) {
             milliseconds = Math.min(milliseconds, MAX_VIBRATE_DURATION_MS);
             mVibrator.vibrate(milliseconds);
         }
 
+        @JavascriptInterface
         @SuppressWarnings("unused")
         public void vibrate(long[] pattern, int repeat) {
             for (int i = 0; i < pattern.length; ++i) {
@@ -336,6 +340,7 @@ public class AccessibilityInjector {
             mVibrator.vibrate(pattern, repeat);
         }
 
+        @JavascriptInterface
         @SuppressWarnings("unused")
         public void cancel() {
             mVibrator.cancel();
@@ -352,16 +357,19 @@ public class AccessibilityInjector {
             mTextToSpeech = new TextToSpeech(context, null, null);
         }
 
+        @JavascriptInterface
         @SuppressWarnings("unused")
         public boolean isSpeaking() {
             return mTextToSpeech.isSpeaking();
         }
 
+        @JavascriptInterface
         @SuppressWarnings("unused")
         public int speak(String text, int queueMode, HashMap<String, String> params) {
             return mTextToSpeech.speak(text, queueMode, params);
         }
 
+        @JavascriptInterface
         @SuppressWarnings("unused")
         public int stop() {
             return mTextToSpeech.stop();