Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(42)

Issue 12290005: Android ScreenInfo size in DIP instead of device pixels. (Closed)

Created:
7 years, 10 months ago by johnme
Modified:
7 years, 10 months ago
CC:
chromium-reviews, yusukes+watch_chromium.org, jam, penghuang+watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, James Su
Visibility:
Public.

Description

Android ScreenInfo size in DIP instead of device pixels. So far, Chrome for Android has always passed around screen sizes in physical device pixels instead of DIPs (density independent pixels). Most other platforms (e.g. Chrome OS) are sizing everything in DIPs (except in places like the compositor that actually do painting). This patch make Chrome for Android also size the screen in DIPs. This is visible all the way to JavaScript/CSS: - JS screen.{width,availWidth} and CSS @media device-width - use: width() and availWidth() in third_party/WebKit/Source/WebCore/page/Screen.cpp and: device_widthMediaFeatureEval in third_party/WebKit/Source/WebCore/css/MediaQueryEvaluator.cpp - that use: screenRect and screenAvailableRect in third_party/WebKit/ Source/WebCore/platform/chromium/PlatformScreenChromium.cpp - that use: screenInfo() in third_party/WebKit/ Source/WebKit/chromium/public/WebWidgetClient.h - implemented by: content/renderer/render_widget.cc - whose screen_info_ member is initialized by the constructor - with a value from: GetWebScreenInfo(WebKit::WebScreenInfo* result) in content/browser/renderer_host/render_widget_host_impl.cc - that uses: GetScreenInfo(WebKit::WebScreenInfo* result) in content/browser/renderer_host/render_widget_host_view_android.cc - that uses: GetDefaultScreenInfo(WebKit::WebScreenInfo* results) in content/browser/renderer_host/render_widget_host_view_android.cc - that before this patch, used: GetDisplayWidth() in ui/gfx/android/device_display_info.cc which called out to JNI - but after this patch, uses: bounds() in ui/gfx/display.h - whose bounds_ member is initialized by the constructor - with a value from: GetPrimaryDisplay() in ui/gfx/screen_android.cc - which also uses: GetDisplayWidth() in ui/gfx/android/device_display_info.cc but first divides by device_scale_factor Hence JS screen.{width,availWidth} and CSS @media device-width now return values in DIPs (which is good) instead of physical device pixels (which was bad). Miraculously, this change doesn't seem to have broken anything in Chrome for Android, though I'm still awaiting the try results ;-) BUG=168325 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=183061

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -7 lines) Patch
M content/browser/renderer_host/render_widget_host_view_android.cc View 2 chunks +10 lines, -7 lines 1 comment Download

Messages

Total messages: 5 (0 generated)
aelias_OOO_until_Jul13
lgtm. We're already transitioned to DIP for most of the stack, this is just a ...
7 years, 10 months ago (2013-02-15 21:04:08 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/johnme@chromium.org/12290005/1
7 years, 10 months ago (2013-02-17 20:17:23 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/johnme@chromium.org/12290005/1
7 years, 10 months ago (2013-02-18 00:18:41 UTC) #3
commit-bot: I haz the power
Change committed as 183061
7 years, 10 months ago (2013-02-18 04:47:56 UTC) #4
Nico
7 years, 10 months ago (2013-02-18 12:01:42 UTC) #5
Message was sent while issue was closed.
Nice!

Powered by Google App Engine
This is Rietveld 408576698