[AiS] Java bridge for AnswersImageService

chrome/browser/android/omnibox/answers_image_bridge.cc

+// 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.
+
+#include "chrome/browser/android/omnibox/answers_image_bridge.h"
+
+#include <jni.h>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/bind.h"
+#include "chrome/browser/autocomplete/answers_image_service.h"
+#include "chrome/browser/autocomplete/answers_image_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_android.h"
+#include "jni/AnswersImage_jni.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/android/java_bitmap.h"
+#include "url/gurl.h"
+
+using base::android::ScopedJavaLocalRef;
+using base::android::ConvertUTF8ToJavaString;
+
+namespace {
+
+class AnswersImageObserverAndroid : public AnswersImageObserver {
+ public:
+  explicit AnswersImageObserverAndroid(jobject java_answers_image_observer)
+      : java_answers_image_observer_(java_answers_image_observer) {}
+
+  virtual ~AnswersImageObserverAndroid() {}
+
+  // AnswersImageObserver:
+  virtual void OnImageChanged(const SkBitmap& answers_image) OVERRIDE {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    ScopedJavaLocalRef<jobject> java_bitmap =
+        gfx::ConvertToJavaBitmap(&answers_image);
+    Java_AnswersImageObserver_onAnswersImageChanged(
+        env, java_answers_image_observer_, java_bitmap.obj());
+  }
+
+  virtual void OnRequestFinished(int request_id) OVERRIDE { delete this; }
+
+ private:
+  jobject java_answers_image_observer_;

[Ted C, 2014/06/06 01:49:23]

Instead of a raw jobject, can you store this as a
base::android::ScopedJavaGlobalRef?

I'm surprised this works actually as the local reference to the jobject should
have been lost when returning from RequestAnaswersImage.

Here's an pretty concise description of what these mean if you're morbidly
interested:
http://developer.android.com/training/articles/perf-jni.html#local_and_global...

[groby-ooo-7-16, 2014/06/13 23:52:21]

I _am_ morbidly interested. Thanks for the pointer. And done :)

+};
+
+}  // namespace
+
+static void CancelAnswersImageRequest(JNIEnv* env,
+                                      jclass,
+                                      jobject java_profile,
+                                      jint java_request_id) {
+  Profile* profile = ProfileAndroid::FromProfileAndroid(java_profile);
+  if (profile) {
+    AnswersImageService* answers_image_service =
+        AnswersImageServiceFactory::GetForBrowserContext(profile);
+    answers_image_service->CancelRequest(java_request_id);
+  }
+}
+
+static jobject RequestAnswersImage(JNIEnv* env,
+                                   jclass,
+                                   jobject java_profile,
+                                   jstring java_url,
+                                   jobject java_callback) {
+  Profile* profile = ProfileAndroid::FromProfileAndroid(java_profile);
+  if (profile) {

[Ted C, 2014/06/06 01:49:24]

I would just dcheck profile here.  People shouldn't be calling these apis with
bad profiles.  Same above.

[groby-ooo-7-16, 2014/06/13 23:52:21]

Done.

+    AnswersImageService* answers_image_service =
+        AnswersImageServiceFactory::GetForBrowserContext(profile);
+    std::string url;
+    base::android::ConvertJavaStringToUTF8(env, java_url, &url);
+    int request_id = answers_image_service->RequestImage(
+        GURL(url), new AnswersImageObserverAndroid(java_callback));
+    return Java_AnswersImage_createAnswersImageRequest(
+               env, java_callback, request_id).Release();

[Ted C, 2014/06/06 01:49:23]

indented too much

[groby-ooo-7-16, 2014/06/13 23:52:21]

That's what 'git cl format' believes to be correct. If it deeply bothers you,
I'll reformat - but I prefer not fighting the automated formatter on each
check-in.

[Ted C, 2014/06/17 20:38:15]

Nothing about this indenting looks sane to me.  If this is what clang-format is
doing, then this very much looks like a bug in the formatter.

I'd rather not let random indenting permeate the code, so I think you should
file this as a bug against clang format.

Bug link:
https://code.google.com/p/chromium/issues/entry?comment=clang-format%20produc...

[groby-ooo-7-16, 2014/06/17 23:46:20]

It _is_ somewhat sane in that the style guide doesn't specify what the +4
indentation for the arguments is relative to. I've filed crbug.com/385905, so
let's see how this plays out.

Thankfully, with just returning a requestId, this is not part of the CL any
more.

+  }
+  return NULL;
+}
+
+// static
+bool RegisterAnswersImageBridge(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}