From e9c0ba7ac5987a7cc3ce2bfa2c8441862b77d258 Mon Sep 17 00:00:00 2001 From: "derat@chromium.org" Date: Wed, 15 Jun 2011 15:46:26 +0000 Subject: [PATCH] base: Move UI code out of SysInfo. This moves GetPrimaryDisplayDimensions() and DisplayCount() out of base and into a new DisplayUtils class (currently alongside the metrics code, since that's the only place that they're called). These methods add a GDK dependency that prevents Chrome OS from including process_util (which depends on SysInfo) in its libchrome library. BUG=chromium-os:16153 TEST=moved existing unit tests Review URL: http://codereview.chromium.org/7128001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@89194 0039d316-1c4b-4281-b951-d872f2087c98 --- base/sys_info.h | 7 --- base/sys_info_mac.cc | 44 ---------------- base/sys_info_posix.cc | 28 ---------- base/sys_info_unittest.cc | 13 ----- base/sys_info_win.cc | 14 ----- chrome/browser/metrics/display_utils.h | 24 +++++++++ chrome/browser/metrics/display_utils_mac.cc | 51 +++++++++++++++++++ chrome/browser/metrics/display_utils_posix.cc | 28 ++++++++++ .../browser/metrics/display_utils_unittest.cc | 25 +++++++++ chrome/browser/metrics/display_utils_win.cc | 21 ++++++++ chrome/browser/metrics/metrics_log.cc | 5 +- chrome/chrome_browser.gypi | 4 ++ chrome/chrome_tests.gypi | 1 + 13 files changed, 157 insertions(+), 108 deletions(-) create mode 100644 chrome/browser/metrics/display_utils.h create mode 100644 chrome/browser/metrics/display_utils_mac.cc create mode 100644 chrome/browser/metrics/display_utils_posix.cc create mode 100644 chrome/browser/metrics/display_utils_unittest.cc create mode 100644 chrome/browser/metrics/display_utils_win.cc diff --git a/base/sys_info.h b/base/sys_info.h index 89bd1931eb2f72..0d2377527add1e 100644 --- a/base/sys_info.h +++ b/base/sys_info.h @@ -54,13 +54,6 @@ class BASE_API SysInfo { // across platforms. static std::string CPUArchitecture(); - // Returns the pixel dimensions of the primary display via the - // width and height parameters. - static void GetPrimaryDisplayDimensions(int* width, int* height); - - // Return the number of displays. - static int DisplayCount(); - // Return the smallest amount of memory (in bytes) which the VM system will // allocate. static size_t VMAllocationGranularity(); diff --git a/base/sys_info_mac.cc b/base/sys_info_mac.cc index 6ffb42f86b6ac9..d8210a29993257 100644 --- a/base/sys_info_mac.cc +++ b/base/sys_info_mac.cc @@ -55,48 +55,4 @@ int64 SysInfo::AmountOfPhysicalMemory() { return static_cast(hostinfo.max_mem); } -// static -void SysInfo::GetPrimaryDisplayDimensions(int* width, int* height) { - CGDirectDisplayID main_display = CGMainDisplayID(); - if (width) - *width = CGDisplayPixelsWide(main_display); - if (height) - *height = CGDisplayPixelsHigh(main_display); -} - -// static -int SysInfo::DisplayCount() { - // Don't just return the number of online displays. It includes displays - // that mirror other displays, which are not desired in the count. It's - // tempting to use the count returned by CGGetActiveDisplayList, but active - // displays exclude sleeping displays, and those are desired in the count. - - // It would be ridiculous to have this many displays connected, but - // CGDirectDisplayID is just an integer, so supporting up to this many - // doesn't hurt. - CGDirectDisplayID online_displays[128]; - CGDisplayCount online_display_count = 0; - if (CGGetOnlineDisplayList(arraysize(online_displays), - online_displays, - &online_display_count) != kCGErrorSuccess) { - // 1 is a reasonable assumption. - return 1; - } - - int display_count = 0; - for (CGDisplayCount online_display_index = 0; - online_display_index < online_display_count; - ++online_display_index) { - CGDirectDisplayID online_display = online_displays[online_display_index]; - if (CGDisplayMirrorsDisplay(online_display) == kCGNullDirectDisplay) { - // If this display doesn't mirror any other, include it in the count. - // The primary display in a mirrored set will be counted, but those that - // mirror it will not be. - ++display_count; - } - } - - return display_count; -} - } // namespace base diff --git a/base/sys_info_posix.cc b/base/sys_info_posix.cc index 3da08e1833fabd..d635eb4ada73c9 100644 --- a/base/sys_info_posix.cc +++ b/base/sys_info_posix.cc @@ -12,10 +12,6 @@ #include #include -#if !defined(OS_MACOSX) -#include -#endif - #include "base/basictypes.h" #include "base/file_util.h" #include "base/logging.h" @@ -78,30 +74,6 @@ std::string SysInfo::CPUArchitecture() { return std::string(info.machine); } -#if !defined(OS_MACOSX) -// static -void SysInfo::GetPrimaryDisplayDimensions(int* width, int* height) { - // Note that Bad Things Happen if this isn't called from the UI thread, - // but also that there's no way to check that from here. :( - GdkScreen* screen = gdk_screen_get_default(); - if (width) - *width = gdk_screen_get_width(screen); - if (height) - *height = gdk_screen_get_height(screen); -} - -// static -int SysInfo::DisplayCount() { - // Note that Bad Things Happen if this isn't called from the UI thread, - // but also that there's no way to check that from here. :( - - // This query is kinda bogus for Linux -- do we want number of X screens? - // The number of monitors Xinerama has? We'll just use whatever GDK uses. - GdkScreen* screen = gdk_screen_get_default(); - return gdk_screen_get_n_monitors(screen); -} -#endif - // static size_t SysInfo::VMAllocationGranularity() { return getpagesize(); diff --git a/base/sys_info_unittest.cc b/base/sys_info_unittest.cc index 06f21cc1983835..57eb905daba759 100644 --- a/base/sys_info_unittest.cc +++ b/base/sys_info_unittest.cc @@ -42,19 +42,6 @@ TEST_F(SysInfoTest, OperatingSystemVersionNumbers) { } #endif -TEST_F(SysInfoTest, GetPrimaryDisplayDimensions) { - // We aren't actually testing that it's correct, just that it's sane. - int width, height; - base::SysInfo::GetPrimaryDisplayDimensions(&width, &height); - EXPECT_GE(width, 10); - EXPECT_GE(height, 10); -} - -TEST_F(SysInfoTest, DisplayCount) { - // We aren't actually testing that it's correct, just that it's sane. - EXPECT_GE(base::SysInfo::DisplayCount(), 1); -} - #if defined(OS_CHROMEOS) TEST_F(SysInfoTest, GoogleChromeOSVersionNumbers) { int32 os_major_version = -1; diff --git a/base/sys_info_win.cc b/base/sys_info_win.cc index 045d516c3fdc77..eb1ccbb8df1d2a 100644 --- a/base/sys_info_win.cc +++ b/base/sys_info_win.cc @@ -76,20 +76,6 @@ std::string SysInfo::CPUArchitecture() { return "x86"; } -// static -void SysInfo::GetPrimaryDisplayDimensions(int* width, int* height) { - if (width) - *width = GetSystemMetrics(SM_CXSCREEN); - - if (height) - *height = GetSystemMetrics(SM_CYSCREEN); -} - -// static -int SysInfo::DisplayCount() { - return GetSystemMetrics(SM_CMONITORS); -} - // static size_t SysInfo::VMAllocationGranularity() { return win::OSInfo::GetInstance()->allocation_granularity(); diff --git a/chrome/browser/metrics/display_utils.h b/chrome/browser/metrics/display_utils.h new file mode 100644 index 00000000000000..c512148879d58a --- /dev/null +++ b/chrome/browser/metrics/display_utils.h @@ -0,0 +1,24 @@ +// Copyright (c) 2011 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. + +#ifndef CHROME_BROWSER_METRICS_DISPLAY_UTILS_H_ +#define CHROME_BROWSER_METRICS_DISPLAY_UTILS_H_ +#pragma once + +#include "base/basictypes.h" + +class DisplayUtils { + public: + // Returns the pixel dimensions of the primary display via the + // width and height parameters. + static void GetPrimaryDisplayDimensions(int* width, int* height); + + // Returns the number of displays. + static int GetDisplayCount(); + + private: + DISALLOW_COPY_AND_ASSIGN(DisplayUtils); +}; + +#endif // CHROME_BROWSER_METRICS_DISPLAY_UTILS_H_ diff --git a/chrome/browser/metrics/display_utils_mac.cc b/chrome/browser/metrics/display_utils_mac.cc new file mode 100644 index 00000000000000..dd0fd305c77987 --- /dev/null +++ b/chrome/browser/metrics/display_utils_mac.cc @@ -0,0 +1,51 @@ +// Copyright (c) 2011 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/metrics/display_utils.h" + +#include + +// static +void DisplayUtils::GetPrimaryDisplayDimensions(int* width, int* height) { + CGDirectDisplayID main_display = CGMainDisplayID(); + if (width) + *width = CGDisplayPixelsWide(main_display); + if (height) + *height = CGDisplayPixelsHigh(main_display); +} + +// static +int DisplayUtils::GetDisplayCount() { + // Don't just return the number of online displays. It includes displays + // that mirror other displays, which are not desired in the count. It's + // tempting to use the count returned by CGGetActiveDisplayList, but active + // displays exclude sleeping displays, and those are desired in the count. + + // It would be ridiculous to have this many displays connected, but + // CGDirectDisplayID is just an integer, so supporting up to this many + // doesn't hurt. + CGDirectDisplayID online_displays[128]; + CGDisplayCount online_display_count = 0; + if (CGGetOnlineDisplayList(arraysize(online_displays), + online_displays, + &online_display_count) != kCGErrorSuccess) { + // 1 is a reasonable assumption. + return 1; + } + + int display_count = 0; + for (CGDisplayCount online_display_index = 0; + online_display_index < online_display_count; + ++online_display_index) { + CGDirectDisplayID online_display = online_displays[online_display_index]; + if (CGDisplayMirrorsDisplay(online_display) == kCGNullDirectDisplay) { + // If this display doesn't mirror any other, include it in the count. + // The primary display in a mirrored set will be counted, but those that + // mirror it will not be. + ++display_count; + } + } + + return display_count; +} diff --git a/chrome/browser/metrics/display_utils_posix.cc b/chrome/browser/metrics/display_utils_posix.cc new file mode 100644 index 00000000000000..e3f566742f6deb --- /dev/null +++ b/chrome/browser/metrics/display_utils_posix.cc @@ -0,0 +1,28 @@ +// Copyright (c) 2011 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/metrics/display_utils.h" + +#include + +#include "content/browser/browser_thread.h" + +// static +void DisplayUtils::GetPrimaryDisplayDimensions(int* width, int* height) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + GdkScreen* screen = gdk_screen_get_default(); + if (width) + *width = gdk_screen_get_width(screen); + if (height) + *height = gdk_screen_get_height(screen); +} + +// static +int DisplayUtils::GetDisplayCount() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + // This query is kinda bogus for Linux -- do we want number of X screens? + // The number of monitors Xinerama has? We'll just use whatever GDK uses. + GdkScreen* screen = gdk_screen_get_default(); + return gdk_screen_get_n_monitors(screen); +} diff --git a/chrome/browser/metrics/display_utils_unittest.cc b/chrome/browser/metrics/display_utils_unittest.cc new file mode 100644 index 00000000000000..11ef45eb7d6232 --- /dev/null +++ b/chrome/browser/metrics/display_utils_unittest.cc @@ -0,0 +1,25 @@ +// Copyright (c) 2011 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 "base/message_loop.h" +#include "chrome/browser/metrics/display_utils.h" +#include "content/browser/browser_thread.h" +#include "testing/gtest/include/gtest/gtest.h" + +TEST(DisplayUtilsTest, GetPrimaryDisplayDimensions) { + MessageLoop message_loop; + BrowserThread ui_thread(BrowserThread::UI, &message_loop); + int width = 0, height = 0; + // We aren't actually testing that it's correct, just that it's sane. + DisplayUtils::GetPrimaryDisplayDimensions(&width, &height); + EXPECT_GE(width, 10); + EXPECT_GE(height, 10); +} + +TEST(DisplayUtilsTest, GetDisplayCount) { + MessageLoop message_loop; + BrowserThread ui_thread(BrowserThread::UI, &message_loop); + // We aren't actually testing that it's correct, just that it's sane. + EXPECT_GE(DisplayUtils::GetDisplayCount(), 1); +} diff --git a/chrome/browser/metrics/display_utils_win.cc b/chrome/browser/metrics/display_utils_win.cc new file mode 100644 index 00000000000000..5b459c860ea777 --- /dev/null +++ b/chrome/browser/metrics/display_utils_win.cc @@ -0,0 +1,21 @@ +// Copyright (c) 2011 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/metrics/display_utils.h" + +#include + +// static +void DisplayUtils::GetPrimaryDisplayDimensions(int* width, int* height) { + if (width) + *width = GetSystemMetrics(SM_CXSCREEN); + + if (height) + *height = GetSystemMetrics(SM_CYSCREEN); +} + +// static +int DisplayUtils::GetDisplayCount() { + return GetSystemMetrics(SM_CMONITORS); +} diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc index 20b18440b80f2a..c826923ef5bb0e 100644 --- a/chrome/browser/metrics/metrics_log.cc +++ b/chrome/browser/metrics/metrics_log.cc @@ -19,6 +19,7 @@ #include "chrome/browser/autocomplete/autocomplete.h" #include "chrome/browser/autocomplete/autocomplete_match.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/metrics/display_utils.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/common/chrome_version_info.h" #include "chrome/common/logging_chrome.h" @@ -350,10 +351,10 @@ void MetricsLog::RecordEnvironment( OPEN_ELEMENT_FOR_SCOPE("display"); int width = 0; int height = 0; - base::SysInfo::GetPrimaryDisplayDimensions(&width, &height); + DisplayUtils::GetPrimaryDisplayDimensions(&width, &height); WriteIntAttribute("xsize", width); WriteIntAttribute("ysize", height); - WriteIntAttribute("screens", base::SysInfo::DisplayCount()); + WriteIntAttribute("screens", DisplayUtils::GetDisplayCount()); } { diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index fe5c0587c91c79..f05bd30e00ddaf 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1360,6 +1360,9 @@ 'browser/memory_details_win.cc', 'browser/memory_purger.cc', 'browser/memory_purger.h', + 'browser/metrics/display_utils_mac.cc', + 'browser/metrics/display_utils_posix.cc', + 'browser/metrics/display_utils_win.cc', 'browser/metrics/field_trial_synchronizer.cc', 'browser/metrics/field_trial_synchronizer.h', 'browser/metrics/histogram_synchronizer.cc', @@ -3774,6 +3777,7 @@ 'browser/importer/nss_decryptor_system_nss.cc', 'browser/importer/nss_decryptor_system_nss.h', 'browser/jankometer.cc', + 'browser/metrics/display_utils_posix.cc', 'browser/password_manager/login_database_posix.cc', 'browser/power_save_blocker_stub.cc', 'browser/ui/browser_list_stub.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index e09fd496afe324..6fada41bb3595a 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1456,6 +1456,7 @@ 'browser/instant/promo_counter_unittest.cc', 'browser/internal_auth_unittest.cc', 'browser/mach_broker_mac_unittest.cc', + 'browser/metrics/display_utils_unittest.cc', 'browser/metrics/metrics_log_unittest.cc', 'browser/metrics/metrics_response_unittest.cc', 'browser/metrics/metrics_service_unittest.cc',