Reland of Android: support multiple displays on C++ side

ui/display/android/screen_android.cc

Index: ui/display/android/screen_android.cc
diff --git a/ui/display/android/screen_android.cc b/ui/display/android/screen_android.cc
index 015d68cd4848a310125d559df280bbfa8d32b9be..9b3ca51653d800b385b110b3702053ed0ef6a7af 100644
--- a/ui/display/android/screen_android.cc
+++ b/ui/display/android/screen_android.cc
@@ -2,89 +2,89 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ui/display/screen.h"
+#include "ui/display/android/screen_android.h"
 
 #include "base/logging.h"
-#include "base/macros.h"
-#include "ui/display/display.h"
-#include "ui/gfx/android/device_display_info.h"
-#include "ui/gfx/geometry/size_conversions.h"
+#include "ui/android/window_android.h"
 
 namespace display {
 
-class ScreenAndroid : public Screen {
- public:
-  ScreenAndroid() {}
-
-  gfx::Point GetCursorScreenPoint() override { return gfx::Point(); }
-
-  bool IsWindowUnderCursor(gfx::NativeWindow window) override {
-    NOTIMPLEMENTED();
-    return false;
-  }
-
-  gfx::NativeWindow GetWindowAtScreenPoint(const gfx::Point& point) override {
-    NOTIMPLEMENTED();
-    return NULL;
-  }
-
-  Display GetPrimaryDisplay() const override {
-    gfx::DeviceDisplayInfo device_info;
-    const float device_scale_factor = device_info.GetDIPScale();
-    // Note: GetPhysicalDisplayWidth/Height() does not subtract window
-    // decorations etc. Use it instead of GetDisplayWidth/Height() when
-    // available.
-    const gfx::Rect bounds_in_pixels =
-        gfx::Rect(device_info.GetPhysicalDisplayWidth()
-                      ? device_info.GetPhysicalDisplayWidth()
-                      : device_info.GetDisplayWidth(),
-                  device_info.GetPhysicalDisplayHeight()
-                      ? device_info.GetPhysicalDisplayHeight()
-                      : device_info.GetDisplayHeight());
-    const gfx::Rect bounds_in_dip = gfx::Rect(gfx::ScaleToCeiledSize(
-        bounds_in_pixels.size(), 1.0f / device_scale_factor));
-    Display display(0, bounds_in_dip);
-    if (!Display::HasForceDeviceScaleFactor())
-      display.set_device_scale_factor(device_scale_factor);
-    display.SetRotationAsDegree(device_info.GetRotationDegrees());
-    display.set_color_depth(device_info.GetBitsPerPixel());
-    display.set_depth_per_component(device_info.GetBitsPerComponent());
-    display.set_is_monochrome(device_info.GetBitsPerComponent() == 0);
-    return display;
-  }
-
-  Display GetDisplayNearestWindow(gfx::NativeView view) const override {
-    return GetPrimaryDisplay();
-  }
-
-  Display GetDisplayNearestPoint(const gfx::Point& point) const override {
-    return GetPrimaryDisplay();
-  }
-
-  int GetNumDisplays() const override { return 1; }
-
-  std::vector<Display> GetAllDisplays() const override {
-    return std::vector<Display>(1, GetPrimaryDisplay());
-  }
-
-  Display GetDisplayMatching(const gfx::Rect& match_rect) const override {
-    return GetPrimaryDisplay();
-  }
-
-  void AddObserver(DisplayObserver* observer) override {
-    // no display change on Android.
-  }
-
-  void RemoveObserver(DisplayObserver* observer) override {
-    // no display change on Android.
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ScreenAndroid);
-};
-
 Screen* CreateNativeScreen() {
   return new ScreenAndroid;
 }
 
+ScreenAndroid::ScreenAndroid() {}
+
+ScreenAndroid::~ScreenAndroid() {}
+
+gfx::Point ScreenAndroid::GetCursorScreenPoint() {
+  return gfx::Point();
+}
+
+bool ScreenAndroid::IsWindowUnderCursor(gfx::NativeWindow window) {
+  NOTIMPLEMENTED();
+  return false;
+}
+
+gfx::NativeWindow ScreenAndroid::GetWindowAtScreenPoint(
+    const gfx::Point& point) {
+  NOTIMPLEMENTED();
+  return NULL;
+}
+
+int ScreenAndroid::GetNumDisplays() const {
+  DCHECK_GE(displays_.size(), 1U);
+  return displays_.size();
+}
+
+std::vector<Display> ScreenAndroid::GetAllDisplays() const {
+  DCHECK_GE(displays_.size(), 1U);
+
+  std::vector<Display> result;
+  for (const auto& display : displays_)
+    result.push_back(display.second);
+
+  return result;
+}
+
+Display ScreenAndroid::GetDisplayNearestWindow(gfx::NativeView view) const {
+  DCHECK(view);
+  ui::WindowAndroid* window = view->GetWindowAndroid();
+
+  DCHECK(window);

[boliu, 2016/10/26 17:24:20]

this may not hold either

+  int display_id = window->display_id();
+
+  DCHECK_EQ(displays_.count(display_id), 1U);

[Tima Vaisburd, 2016/10/26 02:02:18]

This DCHECK will succeed because DisplayAndroid is always created in the process
of WindowAndroid creation, so when we pass a view, the display is already there.

For other methods like GetAllDisplays() or GetPrimaryDisplay() we might not have
a window for it yet.

Shall we create Displays and DisplayAndroid from these methods? 

[boliu, 2016/10/26 17:24:20]

It's more subtle than that. In theory, it's possible for WindowAndroid to
outlive the "display" it's referring to. (Maybe android has guarantees of when
this happens, but I certainly don't want to rely on that). In java, we just keep
a reference to the DisplayAndroid so that object doesn't die, but it's holding
stale data. But in native code, you won't find a display here, so this DCHECK
won't hold in that case
I don't know what this means

[Tima Vaisburd, 2016/10/26 17:58:10]

I was asking whether in GetPrimaryDisplay() and maybe other methods we need to
call into DisplayAndroidManager through jni, get() the primary display (or all
current displays) there to ensure we added them into both maps, and then return
the corresponding Display(s).

I think this is the way to support GetPrimaryDisplay(), and maybe
GetDisplayCount() and GetAllDisplays() if we need them, and really wanted to
hear your opinion.

[boliu, 2016/10/26 18:24:44]

Primary display should be pushed down as well. And it probably shouldn't change,
at least on android. GetAllDisplay should just loop through what's already
there.

+  return displays_.find(display_id)->second;

[mthiesse, 2016/10/26 14:49:49]

I think we need to guarantee always returning a display here, even if it's the
default, so if for some reason we display_id doesn't match a display in the
list, we should return the default.

[boliu, 2016/10/26 17:24:20]

+1 looks like most desktop implementations fallback to the default display, so
should be ok here

[Tima Vaisburd, 2016/10/27 07:55:58]

Following the offline conversation I add the primary display in
DisplayAndroidManager upon the creation of DisplayAndroidMAnager.java. The
primary display is never removed from our maps. GetDisplayNearestWindow() fall
backs to GetPrimaryDisplay().

+}
+
+Display ScreenAndroid::GetDisplayNearestPoint(const gfx::Point& point) const {
+  NOTIMPLEMENTED();
+  return Display();
+}
+
+Display ScreenAndroid::GetDisplayMatching(const gfx::Rect& match_rect) const {
+  NOTIMPLEMENTED();
+  return Display();
+}
+
+Display ScreenAndroid::GetPrimaryDisplay() const {
+  const int kPrimaryDisplayId = 0;

[boliu, 2016/10/26 18:24:44]

where did we decide 0 is the primary one?

[Tima Vaisburd, 2016/10/27 07:55:58]

Display.DEFAULT_DISPLAY is defined as 0, I added the explanation.

+  DCHECK_EQ(displays_.count(kPrimaryDisplayId), 1U);
+  return displays_.find(kPrimaryDisplayId)->second;
+}
+
+// Native side observers are not implemenetd for Android.
+void ScreenAndroid::AddObserver(DisplayObserver* observer) {}
+
+void ScreenAndroid::RemoveObserver(DisplayObserver* observer) {}
+
+void ScreenAndroid::UpdateDisplay(int display_id, const Display& display) {
+  displays_[display_id] = display;
+}
+
+void ScreenAndroid::RemoveDisplay(int display_id) {
+  displays_.erase(display_id);
+}
+
 }  // namespace display