Make WebView work for external displays (over Presentations).

ui/gfx/android/device_display_info.cc

Index: ui/gfx/android/device_display_info.cc
diff --git a/ui/gfx/android/device_display_info.cc b/ui/gfx/android/device_display_info.cc
index 4d2c80c09d6c1d9c4c87102e0138901222943fb9..33f4bb89f9f595e3391c2a483ea568a6d92764f2 100644
--- a/ui/gfx/android/device_display_info.cc
+++ b/ui/gfx/android/device_display_info.cc
@@ -2,53 +2,101 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "jni/DeviceDisplayInfo_jni.h"
 #include "ui/gfx/android/device_display_info.h"
 
-#include "base/logging.h"
-#include "ui/gfx/android/shared_device_display_info.h"
-
 namespace gfx {
 
+DeviceDisplayInfo::DeviceDisplayInfo(
+    base::android::ScopedJavaLocalRef<jobject> context) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  j_device_info_.Reset(Java_DeviceDisplayInfo_create(env, context.obj()));
+  FetchDisplayInfoFromJava(env);
+}
+
 DeviceDisplayInfo::DeviceDisplayInfo() {
+  JNIEnv* env = base::android::AttachCurrentThread();

[boliu, 2015/07/15 06:38:47]

can use c++11 delegated constructor here I think?

[jdduke (slow), 2015/07/15 16:03:07]

Hmm, it might be nice to have a TODO to get rid of the default constructor.

[gsennton, 2015/07/15 19:26:21]

As in changing to use the WindowAndroid context or to just call the other
constructor with GetApplicationContext explicitly?

[gsennton, 2015/07/15 19:26:21]

Done.

[boliu, 2015/07/16 14:36:56]

Yeah, call other constructor directly

+  j_device_info_.Reset(Java_DeviceDisplayInfo_create(
+      env, base::android::GetApplicationContext()));
+  FetchDisplayInfoFromJava(env);
 }
 
 DeviceDisplayInfo::~DeviceDisplayInfo() {
 }
 
-int DeviceDisplayInfo::GetDisplayHeight() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetDisplayHeight();
+void DeviceDisplayInfo::FetchDisplayInfoFromJava(JNIEnv* env) {
+  UpdateDisplayInfo(

[jdduke (slow), 2015/07/15 16:03:07]

Silly question. Do we really need to hold a reference to a Java object here? It
doesn't look like we really support "live" updating of the object.

Would it be any better to do something like:

Java_DeviceDisplayInfo_requestUpdate(env, context, this);

And the Java side would do something like:

@CalledByNative
private void requestUpdate(context, nativePtr) {
  DeviceDisplayInfo info = new DeviceDisplayInfo();
  nativeUpdate(nativePtr, getFoo(), getBar());
}

Alternatively, you could make the Update() public and allow live updating, but
it looks like you simply recreate the object when necessary so I don't really
see the advantage of storing the temporary Java object.

[jdduke (slow), 2015/07/15 16:13:41]

Not only will that cut out most of the JNI calls, we could also just use a
static temporary Java variable to avoid the allocation for native updates
(assuming we only call this on the UI thread? do we?).

[gsennton, 2015/07/15 19:26:21]

I am not sure what you mean by using a static temporary java variable here (and
how that would save allocations)?

Oh, I am updating DeviceDisplayInfo in window_android.cc in a stupid way (by
creating a new native DeviceDisplayInfo and therefore also a new java object for
every update) -- we could just have a call on the native side updating all the
values through jni calls using the stored java object (unless we prefer your
suggestion)?

[boliu, 2015/07/16 14:36:56]

In the code snippet above, instead of "new DeviceDisplayInfo()" each call and
creating something to be gc-ed, just use a static variable instead. Of course
java DeviceDisplayInfo will need a update method too.

This avoids gc churn.
I'm not sure how this is different from what is currently coded

[jdduke (slow), 2015/07/16 15:09:03]

Right, something like "private static DeviceDisplayInfo
sTemporaryDeviceDisplayInfo". You'd create it on-demand, call an update method
on it with the context, feed the updated values to the native object, then
possibly "release" it to clear out any references. Would probably want to add a
UI thread assert for the method called from native.

[boliu, 2015/07/16 16:27:03]

Actually thinking more about this, having a static DeviceDisplayInfo doesn't
work. The whole point of this exercise is have one DeviceDisplayInfo per
context, to deal with external displays. So it can't just be static.

But update only happen on rotation, which doesn't happen all the time. So a
little gc churn isn't that bad.

[gsennton, 2015/10/20 13:27:12]

We do have a DeviceDisplayInfo stored in WindowAndroid so as long as we make
sure that it updates itself by reusing its stored java object all the places
using WindowAndroid should be fine.

What I meant above by me updating DeviceDisplayInfo in a stupid way in
WindowAndroid is that I create a new DeviceDisplayInfo (both native and java)
object on each update, i.e. I call new DeviceDisplayInfo(Context) instead of
device_display_info.FetchDisplayInfo(...).

We could have a static DeviceDisplayInfo object (or some other static object
used from DeviceDisplayInfo) taking care of the ApplicationContext case (the
default constructor case here), but I think we would have to implement this with
a special case for screen listening just like before this CL?

+      Java_DeviceDisplayInfo_getDisplayHeight(env, j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getDisplayWidth(env, j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getPhysicalDisplayHeight(env,
+                                                      j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getPhysicalDisplayWidth(env, j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getBitsPerPixel(env, j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getBitsPerComponent(env, j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getDIPScale(env, j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getSmallestDIPWidth(env, j_device_info_.obj()),
+      Java_DeviceDisplayInfo_getRotationDegrees(env, j_device_info_.obj()));
+}
+
+void DeviceDisplayInfo::UpdateDisplayInfo(jint display_height,
+                                          jint display_width,
+                                          jint physical_display_height,
+                                          jint physical_display_width,
+                                          jint bits_per_pixel,
+                                          jint bits_per_component,
+                                          jdouble dip_scale,
+                                          jint smallest_dip_width,
+                                          jint rotation_degrees) {
+  display_height_ = static_cast<int>(display_height);
+  display_width_ = static_cast<int>(display_width);
+  physical_display_height_ = static_cast<int>(physical_display_height);
+  physical_display_width_ = static_cast<int>(physical_display_width);
+  bits_per_pixel_ = static_cast<int>(bits_per_pixel);
+  bits_per_component_ = static_cast<int>(bits_per_component);
+  dip_scale_ = static_cast<double>(dip_scale);
+  smallest_dip_width_ = static_cast<int>(smallest_dip_width);
+  rotation_degrees_ = static_cast<int>(rotation_degrees);
+}
+
+int DeviceDisplayInfo::GetDisplayHeight() const {
+  return display_height_;
+}
+
+int DeviceDisplayInfo::GetDisplayWidth() const {
+  return display_width_;
 }
 
-int DeviceDisplayInfo::GetDisplayWidth() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetDisplayWidth();
+int DeviceDisplayInfo::GetPhysicalDisplayHeight() const {
+  return physical_display_height_;
 }
 
-int DeviceDisplayInfo::GetPhysicalDisplayHeight() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetPhysicalDisplayHeight();
+int DeviceDisplayInfo::GetPhysicalDisplayWidth() const {
+  return physical_display_width_;
 }
 
-int DeviceDisplayInfo::GetPhysicalDisplayWidth() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetPhysicalDisplayWidth();
+int DeviceDisplayInfo::GetBitsPerPixel() const {
+  return bits_per_pixel_;
 }
 
-int DeviceDisplayInfo::GetBitsPerPixel() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetBitsPerPixel();
+int DeviceDisplayInfo::GetBitsPerComponent() const {
+  return bits_per_component_;
 }
 
-int DeviceDisplayInfo::GetBitsPerComponent() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetBitsPerComponent();
+double DeviceDisplayInfo::GetDIPScale() const {
+  return dip_scale_;
 }
 
-double DeviceDisplayInfo::GetDIPScale() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetDIPScale();
+int DeviceDisplayInfo::GetSmallestDIPWidth() const {
+  return smallest_dip_width_;
 }
 
-int DeviceDisplayInfo::GetSmallestDIPWidth() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetSmallestDIPWidth();
+int DeviceDisplayInfo::GetRotationDegrees() const {
+  return rotation_degrees_;
 }
 
-int DeviceDisplayInfo::GetRotationDegrees() {
-  return SharedDeviceDisplayInfo::GetInstance()->GetRotationDegrees();
+// static
+bool DeviceDisplayInfo::RegisterDeviceDisplayInfo(JNIEnv* env) {
+  return RegisterNativesImpl(env);
 }
 
 }  // namespace gfx