Initialize device scale factor correctly on Android

content/renderer/render_widget.cc

 // 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.
 
 #include "content/renderer/render_widget.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
@@ skipping 17 matching lines @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebPopupMenu.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebPopupMenuInfo.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebRange.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebScreenInfo.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebPoint.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebRect.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebSize.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
 #include "third_party/skia/include/core/SkShader.h"
 #include "ui/base/ui_base_switches.h"
+#include "ui/gfx/display.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/gl/gl_switches.h"
 #include "ui/surface/transport_dib.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/plugins/npapi/webplugin.h"
 #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 
 #if defined(OS_POSIX)
@@ skipping 63 matching lines @@
       screen_info_(screen_info),
       device_scale_factor_(1),
       throttle_input_events_(true),
       next_smooth_scroll_gesture_id_(0),
       is_threaded_compositing_enabled_(false) {
   if (!swapped_out)
     RenderProcess::current()->AddRefProcess();
   DCHECK(RenderThread::Get());
   has_disable_gpu_vsync_switch_ = CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kDisableGpuVsync);
-#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
-  device_scale_factor_ = screen_info.verticalDPI / kStandardDPI;
-  // Unless an explicit scale factor was provided for testing, ensure the scale
-  // is integral.
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kForceDeviceScaleFactor))
-    device_scale_factor_ = static_cast<int>(device_scale_factor_);
-  device_scale_factor_ = std::max(1.0f, device_scale_factor_);
-#endif
   is_threaded_compositing_enabled_ =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableThreadedCompositing);
+
+  scoped_ptr<gfx::Display> display(new gfx::Display());
+  display->SetScaleAndBounds(

[johnme, 2012/11/06 15:45:34]

Would it make sense to make a new constructor for Display which accepts a
screen_info and automatically calls SetScaleAndBounds with this scale factor, so
we can move even more logic out of render_widget? jamesr? oshima?

[Sami, 2012/11/06 17:08:47]

Good idea, that makes things a little cleaner. Someone correct me if I'm wrong,
but since WebScreenInfo is a public WebKit class we can use it from
gfx::Display.

+      screen_info.verticalDPI / kStandardDPI, display->bounds());

[johnme, 2012/11/06 15:45:34]

Presumably this works on Android because in practice we initialize
screen_info.verticalDPI to device_info->GetDPIScale() * 160? We definitely
shouldn't be relying on DisplayMetrics's xdpi and ydpi, as those are
unreliable/deprecated.

Nit: I'd rather we used horizontalDPI instead of verticalDPI, since one of the
most important uses for the scale is determining the device-width that will be
used in mobile viewports. Alternatively, even better is probably to use
min(horizontalDPI, verticalDPI).

P.S. I notice that Mac and CrOS are using the wrong kStandardDPI; I filed
http://crbug.com/159603 about that (ultimately recommending that we move
chromium away from dealing with dpi to dealing purely in device scale factors).

[Sami, 2012/11/06 17:08:47]

That's right, and based on the discussion it seems like some of the other ports
are doing it as well.
I went for your min(hdpi, vdpi) suggestion. I checked that Android, Mac and CrOS
seem to be initializing both horizontal and vertical DPI to the same value, so
in practice this probably won't matter.
 

+  device_scale_factor_ = display->device_scale_factor();
 }
 
 RenderWidget::~RenderWidget() {
   DCHECK(!webwidget_) << "Leaking our WebWidget!";
   STLDeleteElements(&updates_pending_swap_);
   if (current_paint_buf_) {
     RenderProcess::current()->ReleaseTransportDIB(current_paint_buf_);
     current_paint_buf_ = NULL;
   }
   // If we are swapped out, we have released already.
@@ skipping 1778 matching lines @@
 
 bool RenderWidget::WillHandleMouseEvent(const WebKit::WebMouseEvent& event) {
   return false;
 }
 
 bool RenderWidget::WebWidgetHandlesCompositorScheduling() const {
   return false;
 }
 
 }  // namespace content