Fix an incorrect comment in the Java Bridge when calling array methods

content/browser/renderer_host/java/java_bound_object.cc

 // Copyright (c) 2011 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.
 
 #include "content/browser/renderer_host/java/java_bound_object.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/memory/singleton.h"
 #include "base/string_number_conversions.h"
@@ skipping 135 matching lines @@
       break;
     case JavaType::TypeDouble:
       DOUBLE_TO_NPVARIANT(env->CallDoubleMethodA(object, id, parameters),
                           result);
       break;
     case JavaType::TypeVoid:
       env->CallVoidMethodA(object, id, parameters);
       VOID_TO_NPVARIANT(result);
       break;
     case JavaType::TypeArray:
-      // TODO(steveblock): Handle arrays
+      // LIVECONNECT_COMPLIANCE: Don't call methods that return arrays, to
+      // maintain existing behavior. We should call the method and convert the

[bulach, 2011/11/29 11:11:14]

is the second sentence a TODO? also, I got comments that we should avoid we :)
so this either would be TODO(steveblock) call the method and.. or something like
"The new behavior should call.."

[Steve Block, 2011/11/29 11:18:00]

It't not really a TODO. This file has many comments of this style. They're used
to document areas where we don't follow the LiveConnect spec. The first sentence
is what we do. The second is what we should do to match the spec. We have no
immediate plans to match the spec, but may revisit this in the future.

If using 'we' is discouraged, I could update all the comments to use something
like 'The spec requires calling the method ...'. Can you point me to the
guidance on 'we'?

[bulach, 2011/11/29 11:46:19]

got it, thanks for the clarification!
here's the comment I had:
http://codereview.chromium.org/8586054/diff/2001/base/base.gypi#newcode606

yes, I think the "The spec requires" would make it clearer the distinction
between LIVECONNECT_COMPLIANCE and the spec..

Maybe something like:

LIVECONNECT_COMPLIANCE: Existing behavior, %s.
The spec requires %s.

+      // result to a JavaScript array.
       VOID_TO_NPVARIANT(result);
       break;
     case JavaType::TypeString: {
       ScopedJavaLocalRef<jstring> java_string(env, static_cast<jstring>(
           env->CallObjectMethodA(object, id, parameters)));
       if (!java_string.obj()) {
         // LIVECONNECT_COMPLIANCE: Return undefined to maintain existing
         // behavior. We should return a null string.
         VOID_TO_NPVARIANT(result);
         break;
@@ skipping 438 matching lines @@
   size_t num_methods = env->GetArrayLength(methods.obj());
   DCHECK(num_methods) << "Java objects always have public methods";
   for (size_t i = 0; i < num_methods; ++i) {
     ScopedJavaLocalRef<jobject> java_method(
         env,
         env->GetObjectArrayElement(methods.obj(), i));
     JavaMethod* method = new JavaMethod(java_method);
     methods_.insert(std::make_pair(method->name(), method));
   }
 }