Skip to content

Commit

Permalink
base: Move UI code out of SysInfo.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
derat@chromium.org committed Jun 15, 2011
1 parent 32d7925 commit e9c0ba7
Show file tree
Hide file tree
Showing 13 changed files with 157 additions and 108 deletions.
7 changes: 0 additions & 7 deletions base/sys_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
44 changes: 0 additions & 44 deletions base/sys_info_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,48 +55,4 @@ int64 SysInfo::AmountOfPhysicalMemory() {
return static_cast<int64>(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
28 changes: 0 additions & 28 deletions base/sys_info_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@
#include <sys/utsname.h>
#include <unistd.h>

#if !defined(OS_MACOSX)
#include <gdk/gdk.h>
#endif

#include "base/basictypes.h"
#include "base/file_util.h"
#include "base/logging.h"
Expand Down Expand Up @@ -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();
Expand Down
13 changes: 0 additions & 13 deletions base/sys_info_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 0 additions & 14 deletions base/sys_info_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
24 changes: 24 additions & 0 deletions chrome/browser/metrics/display_utils.h
Original file line number Diff line number Diff line change
@@ -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_
51 changes: 51 additions & 0 deletions chrome/browser/metrics/display_utils_mac.cc
Original file line number Diff line number Diff line change
@@ -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 <ApplicationServices/ApplicationServices.h>

// 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;
}
28 changes: 28 additions & 0 deletions chrome/browser/metrics/display_utils_posix.cc
Original file line number Diff line number Diff line change
@@ -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 <gdk/gdk.h>

#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);
}
25 changes: 25 additions & 0 deletions chrome/browser/metrics/display_utils_unittest.cc
Original file line number Diff line number Diff line change
@@ -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);
}
21 changes: 21 additions & 0 deletions chrome/browser/metrics/display_utils_win.cc
Original file line number Diff line number Diff line change
@@ -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 <windows.h>

// 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);
}
5 changes: 3 additions & 2 deletions chrome/browser/metrics/metrics_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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());
}

{
Expand Down
4 changes: 4 additions & 0 deletions chrome/chrome_browser.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions chrome/chrome_tests.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit e9c0ba7

Please sign in to comment.