From ab0da4d961e8d5dee37f6772fc9cbc0879db0bf5 Mon Sep 17 00:00:00 2001
From: "oshima@chromium.org"
<oshima@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Mon, 1 Jul 2013 21:25:36 +0000
Subject: Update display bounds only once before calling observer Use correct
order to make display id pair in UpdateSecondaryDisplayBoundsForLayout.
BUG=253991
TEST=covered by test. no new feature. all tests still pass.
Review URL: https://chromiumcodereview.appspot.com/18117010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209502 0039d316-1c4b-4281-b951-d872f2087c98
---
ash/display/display_controller.cc | 43 ++++++++++++++++++---------------
ash/display/display_controller.h | 12 ++++++---
ash/display/display_manager.cc | 9 +++++--
ash/display/display_manager_unittest.cc | 25 ++++++++++++++-----
4 files changed, 58 insertions(+), 31 deletions(-)
(limited to 'ash')
diff --git a/ash/display/display_controller.cc b/ash/display/display_controller.cc
index e7b66ad..e8186db 100644
--- a/ash/display/display_controller.cc
+++ b/ash/display/display_controller.cc
@@ -300,11 +300,11 @@ void DisplayController::InitPrimaryDisplay() {
GetDisplayManager()->GetPrimaryDisplayCandidate();
primary_display_id = primary_candidate->id();
AddRootWindowForDisplay(*primary_candidate);
- UpdateDisplayBoundsForLayout();
}
void DisplayController::InitSecondaryDisplays() {
internal::DisplayManager* display_manager = GetDisplayManager();
+ UpdateDisplayBoundsForLayout();
for (size_t i = 0; i < display_manager->GetNumDisplays(); ++i) {
const gfx::Display* display = display_manager->GetDisplayAt(i);
if (primary_display_id != display->id()) {
@@ -313,7 +313,6 @@ void DisplayController::InitSecondaryDisplays() {
}
}
if (display_manager->GetNumDisplays() > 1) {
- UpdateDisplayBoundsForLayout();
DisplayIdPair pair = GetCurrentDisplayIdPair();
DisplayLayout layout = GetCurrentDisplayLayout();
SetPrimaryDisplayId(
@@ -420,12 +419,13 @@ void DisplayController::SetLayoutForCurrentDisplays(
to_set.primary_id = primary.id();
paired_layouts_[pair] = to_set;
NotifyDisplayConfigurationChanging();
+ // TODO(oshima): Call UpdateDisplays instead.
UpdateDisplayBoundsForLayout();
NotifyDisplayConfigurationChanged();
}
}
-DisplayLayout DisplayController::GetCurrentDisplayLayout() const {
+DisplayLayout DisplayController::GetCurrentDisplayLayout() {
DCHECK_EQ(2U, GetDisplayManager()->num_connected_displays());
// Invert if the primary was swapped.
if (GetDisplayManager()->num_connected_displays() > 1) {
@@ -433,7 +433,9 @@ DisplayLayout DisplayController::GetCurrentDisplayLayout() const {
return ComputeDisplayLayoutForDisplayIdPair(pair);
}
// On release build, just fallback to default instead of blowing up.
- return default_display_layout_;
+ DisplayLayout layout = default_display_layout_;
+ layout.primary_id = primary_display_id;
+ return layout;
}
DisplayIdPair DisplayController::GetCurrentDisplayIdPair() const {
@@ -455,10 +457,11 @@ DisplayIdPair DisplayController::GetCurrentDisplayIdPair() const {
}
DisplayLayout DisplayController::GetRegisteredDisplayLayout(
- const DisplayIdPair& pair) const {
+ const DisplayIdPair& pair) {
std::map<DisplayIdPair, DisplayLayout>::const_iterator iter =
paired_layouts_.find(pair);
- return iter != paired_layouts_.end() ? iter->second : default_display_layout_;
+ return
+ iter != paired_layouts_.end() ? iter->second : CreateDisplayLayout(pair);
}
void DisplayController::ToggleMirrorMode() {
@@ -658,22 +661,15 @@ void DisplayController::UpdateMouseCursor(const gfx::Point& point_in_native) {
}
void DisplayController::OnDisplayBoundsChanged(const gfx::Display& display) {
- if (limiter_)
- limiter_->SetThrottleTimeout(kAfterDisplayChangeThrottleTimeoutMs);
const internal::DisplayInfo& display_info =
GetDisplayManager()->GetDisplayInfo(display.id());
DCHECK(!display_info.bounds_in_pixel().IsEmpty());
-
- UpdateDisplayBoundsForLayout();
aura::RootWindow* root = root_windows_[display.id()];
root->SetHostBounds(display_info.bounds_in_pixel());
SetDisplayPropertiesOnHostWindow(root, display);
}
void DisplayController::OnDisplayAdded(const gfx::Display& display) {
- if (limiter_)
- limiter_->SetThrottleTimeout(kAfterDisplayChangeThrottleTimeoutMs);
-
if (primary_root_window_for_replace_) {
DCHECK(root_windows_.empty());
primary_display_id = display.id();
@@ -681,7 +677,6 @@ void DisplayController::OnDisplayAdded(const gfx::Display& display) {
primary_root_window_for_replace_->SetProperty(
internal::kDisplayIdKey, display.id());
primary_root_window_for_replace_ = NULL;
- UpdateDisplayBoundsForLayout();
const internal::DisplayInfo& display_info =
GetDisplayManager()->GetDisplayInfo(display.id());
root_windows_[display.id()]->SetHostBounds(
@@ -691,15 +686,11 @@ void DisplayController::OnDisplayAdded(const gfx::Display& display) {
primary_display_id = display.id();
DCHECK(!root_windows_.empty());
aura::RootWindow* root = AddRootWindowForDisplay(display);
- UpdateDisplayBoundsForLayout();
Shell::GetInstance()->InitRootWindowForSecondaryDisplay(root);
}
}
void DisplayController::OnDisplayRemoved(const gfx::Display& display) {
- if (limiter_)
- limiter_->SetThrottleTimeout(kAfterDisplayChangeThrottleTimeoutMs);
-
aura::RootWindow* root_to_delete = root_windows_[display.id()];
DCHECK(root_to_delete) << display.ToString();
@@ -796,13 +787,17 @@ void DisplayController::NotifyDisplayConfigurationChanging() {
void DisplayController::NotifyDisplayConfigurationChanged() {
if (in_bootstrap())
return;
+
+ if (limiter_)
+ limiter_->SetThrottleTimeout(kAfterDisplayChangeThrottleTimeoutMs);
+
focus_activation_store_->Restore();
internal::DisplayManager* display_manager = GetDisplayManager();
if (display_manager->num_connected_displays() > 1) {
DisplayIdPair pair = GetCurrentDisplayIdPair();
if (paired_layouts_.find(pair) == paired_layouts_.end())
- paired_layouts_[pair] = default_display_layout_;
+ CreateDisplayLayout(pair);
paired_layouts_[pair].mirrored = display_manager->IsMirrored();
if (Shell::GetScreen()->GetNumDisplays() > 1 ) {
int64 primary_id = paired_layouts_[pair].primary_id;
@@ -838,7 +833,7 @@ void DisplayController::OnFadeOutForSwapDisplayFinished() {
}
DisplayLayout DisplayController::ComputeDisplayLayoutForDisplayIdPair(
- const DisplayIdPair& pair) const {
+ const DisplayIdPair& pair) {
DisplayLayout layout = GetRegisteredDisplayLayout(pair);
int64 primary_id = layout.primary_id;
// TODO(oshima): replace this with DCHECK.
@@ -865,4 +860,12 @@ void DisplayController::UpdateHostWindowNames() {
#endif
}
+DisplayLayout DisplayController::CreateDisplayLayout(
+ const DisplayIdPair& pair) {
+ DisplayLayout layout = default_display_layout_;
+ layout.primary_id = pair.first;
+ paired_layouts_[pair] = layout;
+ return layout;
+}
+
} // namespace ash
diff --git a/ash/display/display_controller.h b/ash/display/display_controller.h
index b1cdd08..299be14 100644
--- a/ash/display/display_controller.h
+++ b/ash/display/display_controller.h
@@ -44,6 +44,7 @@ class RootWindowController;
// DisplayController owns and maintains RootWindows for each attached
// display, keeping them in sync with display configuration changes.
+// TODO(oshima): Factor out the layout registration class.
class ASH_EXPORT DisplayController : public gfx::DisplayObserver {
public:
class ASH_EXPORT Observer {
@@ -140,13 +141,15 @@ class ASH_EXPORT DisplayController : public gfx::DisplayObserver {
void SetLayoutForCurrentDisplays(const DisplayLayout& layout);
// Returns the display layout used for current displays.
- DisplayLayout GetCurrentDisplayLayout() const;
+ DisplayLayout GetCurrentDisplayLayout();
// Returns the current display pair.
DisplayIdPair GetCurrentDisplayIdPair() const;
// Returns the display layout registered for the given display id |pair|.
- DisplayLayout GetRegisteredDisplayLayout(const DisplayIdPair& pair) const;
+ // If no layout is registered, it creatas new layout using
+ // |default_display_layout_|.
+ DisplayLayout GetRegisteredDisplayLayout(const DisplayIdPair& pair);
// Checks if the mouse pointer is on one of displays, and moves to
// the center of the nearest display if it's outside of all displays.
@@ -192,10 +195,13 @@ class ASH_EXPORT DisplayController : public gfx::DisplayObserver {
// with display swapping applied. That is, this returns
// flipped layout if the displays are swapped.
DisplayLayout ComputeDisplayLayoutForDisplayIdPair(
- const DisplayIdPair& display_pair) const;
+ const DisplayIdPair& display_pair);
void UpdateHostWindowNames();
+ // Creates new layout for display pair from |default_display_layout_|.
+ DisplayLayout CreateDisplayLayout(const DisplayIdPair& display_pair);
+
bool in_bootstrap() const { return in_bootstrap_; }
class DisplayChangeLimiter {
diff --git a/ash/display/display_manager.cc b/ash/display/display_manager.cc
index fec132c..11fb650 100644
--- a/ash/display/display_manager.cc
+++ b/ash/display/display_manager.cc
@@ -200,6 +200,7 @@ void DisplayManager::UpdateDisplayBoundsForLayout(
const DisplayLayout& layout,
const gfx::Display& primary_display,
gfx::Display* secondary_display) {
+ DCHECK_EQ("0,0", primary_display.bounds().origin().ToString());
const gfx::Rect& primary_bounds = primary_display.bounds();
DisplayController::GetPrimaryDisplay().bounds();
@@ -905,7 +906,7 @@ gfx::Display DisplayManager::CreateDisplayFromDisplayInfoById(int64 id) {
// Simply set the origin to (0,0). The primary display's origin is
// always (0,0) and the secondary display's bounds will be updated
- // by |DisplayController::UpdateDisplayBoundsForLayout|.
+ // in |UpdateSecondaryDisplayBoundsForLayout| called in |UpdateDisplay|.
new_display.SetScaleAndBounds(
display_info.device_scale_factor(), gfx::Rect(bounds_in_pixel.size()));
new_display.set_rotation(display_info.rotation());
@@ -919,8 +920,12 @@ bool DisplayManager::UpdateSecondaryDisplayBoundsForLayout(
return false;
DisplayController* controller = Shell::GetInstance()->display_controller();
+ int64 id_at_zero = displays->at(0).id();
DisplayIdPair pair =
- std::make_pair(displays->at(0).id(), displays->at(1).id());
+ (id_at_zero == first_display_id_ ||
+ id_at_zero == gfx::Display::InternalDisplayId()) ?
+ std::make_pair(id_at_zero, displays->at(1).id()) :
+ std::make_pair(displays->at(1).id(), id_at_zero) ;
DisplayLayout layout =
controller->ComputeDisplayLayoutForDisplayIdPair(pair);
diff --git a/ash/display/display_manager_unittest.cc b/ash/display/display_manager_unittest.cc
index 8882af9..f06aa78 100644
--- a/ash/display/display_manager_unittest.cc
+++ b/ash/display/display_manager_unittest.cc
@@ -11,6 +11,7 @@
#include "ash/test/display_manager_test_api.h"
#include "ash/test/mirror_window_test_api.h"
#include "base/format_macros.h"
+#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
#include "ui/aura/env.h"
#include "ui/aura/root_window.h"
@@ -27,6 +28,14 @@ using std::string;
using base::StringPrintf;
+namespace {
+
+std::string ToDisplayName(int64 id) {
+ return "x-" + base::Int64ToString(id);
+}
+
+} // namespace
+
class DisplayManagerTest : public test::AshTestBase,
public gfx::DisplayObserver,
public aura::WindowObserver {
@@ -390,13 +399,13 @@ TEST_F(DisplayManagerTest, TestDeviceScaleOnlyChange) {
}
DisplayInfo CreateDisplayInfo(int64 id, const gfx::Rect& bounds) {
- DisplayInfo info(id, StringPrintf("x-%d", static_cast<int>(id)), false);
+ DisplayInfo info(id, ToDisplayName(id), false);
info.SetBounds(bounds);
return info;
}
TEST_F(DisplayManagerTest, TestNativeDisplaysChanged) {
- const int internal_display_id =
+ const int64 internal_display_id =
test::DisplayManagerTestApi(display_manager()).
SetFirstDisplayAsInternalDisplay();
const int external_id = 10;
@@ -438,20 +447,24 @@ TEST_F(DisplayManagerTest, TestNativeDisplaysChanged) {
EXPECT_FALSE(display_manager()->mirrored_display().is_valid());
EXPECT_EQ(external_id, Shell::GetScreen()->GetPrimaryDisplay().id());
+ EXPECT_EQ(internal_display_id, gfx::Display::InternalDisplayId());
+
// Primary connected, with different bounds.
display_info_list.clear();
display_info_list.push_back(internal_display_info);
display_info_list.push_back(external_display_info);
display_manager()->OnNativeDisplaysChanged(display_info_list);
EXPECT_EQ(2U, display_manager()->GetNumDisplays());
- // need to remember which is primary
+ EXPECT_EQ(internal_display_id, Shell::GetScreen()->GetPrimaryDisplay().id());
+
+ // This combinatino is new, so internal display becomes primary.
EXPECT_EQ("0,0 500x500",
FindDisplayForId(internal_display_id).bounds().ToString());
EXPECT_EQ("1,1 100x100",
FindDisplayInfoForId(10).bounds_in_pixel().ToString());
EXPECT_EQ(2U, display_manager()->num_connected_displays());
EXPECT_FALSE(display_manager()->mirrored_display().is_valid());
- EXPECT_EQ(StringPrintf("x-%d", internal_display_id),
+ EXPECT_EQ(ToDisplayName(internal_display_id),
display_manager()->GetDisplayNameForId(internal_display_id));
// Emulate suspend.
@@ -464,7 +477,7 @@ TEST_F(DisplayManagerTest, TestNativeDisplaysChanged) {
FindDisplayInfoForId(10).bounds_in_pixel().ToString());
EXPECT_EQ(2U, display_manager()->num_connected_displays());
EXPECT_FALSE(display_manager()->mirrored_display().is_valid());
- EXPECT_EQ(StringPrintf("x-%d", internal_display_id),
+ EXPECT_EQ(ToDisplayName(internal_display_id),
display_manager()->GetDisplayNameForId(internal_display_id));
// External display has disconnected then resumed.
@@ -512,7 +525,7 @@ TEST_F(DisplayManagerTest, TestNativeDisplaysChanged) {
EXPECT_TRUE(display_manager()->IsMirrored());
// Test display name.
- EXPECT_EQ(StringPrintf("x-%d", internal_display_id),
+ EXPECT_EQ(ToDisplayName(internal_display_id),
display_manager()->GetDisplayNameForId(internal_display_id));
EXPECT_EQ("x-10", display_manager()->GetDisplayNameForId(10));
EXPECT_EQ("x-11", display_manager()->GetDisplayNameForId(11));
--
cgit v1.1