From 1d5b83105901dbc9b79e52cfff944d32d829e9e3 Mon Sep 17 00:00:00 2001 From: oshima Date: Thu, 8 Jan 2015 17:50:57 -0800 Subject: [PATCH] Cache validation should check it display id if its external display. Chrome may fail to fetch EDID for external display during startup. We need to re-fetch the EDID if the id is zero. BUG=438840, chrome-os-partner:34661 TEST=covered by test. also tested manually on the device. Review URL: https://codereview.chromium.org/843853002 Cr-Commit-Position: refs/heads/master@{#310661} --- .../x11/native_display_event_dispatcher_x11.cc | 9 ++- ...native_display_event_dispatcher_x11_unittest.cc | 66 +++++++++++++++++----- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/ui/display/chromeos/x11/native_display_event_dispatcher_x11.cc b/ui/display/chromeos/x11/native_display_event_dispatcher_x11.cc index 8e61b9b1ae9e..9e206ac7e3e4 100644 --- a/ui/display/chromeos/x11/native_display_event_dispatcher_x11.cc +++ b/ui/display/chromeos/x11/native_display_event_dispatcher_x11.cc @@ -77,9 +77,16 @@ uint32_t NativeDisplayEventDispatcherX11::DispatchEvent( static_cast(x11_output->current_mode()); RRMode mode_id = x11_mode ? x11_mode->mode_id() : None; + // Update if we failed to fetch the external display's ID before. + // Internal display's EDID should always be available. + bool display_id_needs_update = + x11_output->type() != ui::DISPLAY_CONNECTION_TYPE_INTERNAL && + !x11_output->display_id(); + if (x11_output->output() == output_change_event->output) { if (connected && x11_output->crtc() == output_change_event->crtc && - mode_id == output_change_event->mode) { + mode_id == output_change_event->mode && + !display_id_needs_update) { VLOG(1) << "Ignoring event describing already-cached state"; return POST_DISPATCH_PERFORM_DEFAULT; } diff --git a/ui/display/chromeos/x11/native_display_event_dispatcher_x11_unittest.cc b/ui/display/chromeos/x11/native_display_event_dispatcher_x11_unittest.cc index de8779624e9c..eb125f32e635 100644 --- a/ui/display/chromeos/x11/native_display_event_dispatcher_x11_unittest.cc +++ b/ui/display/chromeos/x11/native_display_event_dispatcher_x11_unittest.cc @@ -18,15 +18,18 @@ namespace ui { namespace { -DisplaySnapshotX11* CreateOutput(RROutput output, RRCrtc crtc) { +DisplaySnapshotX11* CreateOutput(int64_t id, + DisplayConnectionType type, + RROutput output, + RRCrtc crtc) { static const DisplayModeX11* kDefaultDisplayMode = new DisplayModeX11(gfx::Size(1, 1), false, 60.0f, 20); DisplaySnapshotX11* snapshot = new DisplaySnapshotX11( - 0, + id, gfx::Point(0, 0), gfx::Size(0, 0), - DISPLAY_CONNECTION_TYPE_UNKNOWN, + type, false, false, std::string(), @@ -40,6 +43,20 @@ DisplaySnapshotX11* CreateOutput(RROutput output, RRCrtc crtc) { return snapshot; } +DisplaySnapshotX11* CreateExternalOutput(RROutput output, RRCrtc crtc) { + return CreateOutput(static_cast(output), + DISPLAY_CONNECTION_TYPE_UNKNOWN, + output, + crtc); +} + +DisplaySnapshotX11* CreateInternalOutput(RROutput output, RRCrtc crtc) { + return CreateOutput(0, + DISPLAY_CONNECTION_TYPE_INTERNAL, + output, + crtc); +} + class TestHelperDelegate : public NativeDisplayDelegateX11::HelperDelegate { public: TestHelperDelegate(); @@ -169,7 +186,7 @@ TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationAfterSecondEvent) { // Simulate addition of the first output to the cached output list. ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); + outputs.push_back(CreateExternalOutput(1, 10)); helper_delegate_->set_cached_outputs(outputs.get()); DispatchOutputChangeEvent(2, 11, 20, true); @@ -178,7 +195,7 @@ TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationAfterSecondEvent) { TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnDisconnect) { ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); + outputs.push_back(CreateExternalOutput(1, 10)); helper_delegate_->set_cached_outputs(outputs.get()); DispatchOutputChangeEvent(1, 10, 20, false); @@ -187,7 +204,7 @@ TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnDisconnect) { TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnModeChange) { ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); + outputs.push_back(CreateExternalOutput(1, 10)); helper_delegate_->set_cached_outputs(outputs.get()); DispatchOutputChangeEvent(1, 10, 21, true); @@ -196,7 +213,7 @@ TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnModeChange) { TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnSecondOutput) { ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); + outputs.push_back(CreateExternalOutput(1, 10)); helper_delegate_->set_cached_outputs(outputs.get()); DispatchOutputChangeEvent(2, 11, 20, true); @@ -205,7 +222,7 @@ TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnSecondOutput) { TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnDifferentCrtc) { ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); + outputs.push_back(CreateExternalOutput(1, 10)); helper_delegate_->set_cached_outputs(outputs.get()); DispatchOutputChangeEvent(1, 11, 20, true); @@ -215,8 +232,8 @@ TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnDifferentCrtc) { TEST_F(NativeDisplayEventDispatcherX11Test, CheckNotificationOnSecondOutputDisconnect) { ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); - outputs.push_back(CreateOutput(2, 11)); + outputs.push_back(CreateExternalOutput(1, 10)); + outputs.push_back(CreateExternalOutput(2, 11)); helper_delegate_->set_cached_outputs(outputs.get()); DispatchOutputChangeEvent(2, 11, 20, false); @@ -226,8 +243,8 @@ TEST_F(NativeDisplayEventDispatcherX11Test, TEST_F(NativeDisplayEventDispatcherX11Test, AvoidDuplicateNotificationOnSecondOutputDisconnect) { ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); - outputs.push_back(CreateOutput(2, 11)); + outputs.push_back(CreateExternalOutput(1, 10)); + outputs.push_back(CreateExternalOutput(2, 11)); helper_delegate_->set_cached_outputs(outputs.get()); DispatchOutputChangeEvent(2, 11, 20, false); @@ -248,8 +265,8 @@ TEST_F(NativeDisplayEventDispatcherX11Test, NativeDisplayEventDispatcherX11::kUseCacheAfterStartupMs / 2 + 1; ScopedVector outputs; - outputs.push_back(CreateOutput(1, 10)); - outputs.push_back(CreateOutput(2, 11)); + outputs.push_back(CreateExternalOutput(1, 10)); + outputs.push_back(CreateExternalOutput(2, 11)); helper_delegate_->set_cached_outputs(outputs.get()); EXPECT_EQ(0, helper_delegate_->num_calls_notify_observers()); @@ -288,4 +305,25 @@ TEST_F(NativeDisplayEventDispatcherX11Test, EXPECT_EQ(4, helper_delegate_->num_calls_notify_observers()); } +TEST_F(NativeDisplayEventDispatcherX11Test, + UpdateMissingExternalDisplayId) { + ScopedVector outputs; + outputs.push_back(CreateInternalOutput(1, 10)); + helper_delegate_->set_cached_outputs(outputs.get()); + + ASSERT_EQ(0, helper_delegate_->num_calls_notify_observers()); + + // Internal display's ID can be zero and not updated. + DispatchOutputChangeEvent(1, 10, 20, true); + EXPECT_EQ(0, helper_delegate_->num_calls_notify_observers()); + + outputs.clear(); + outputs.push_back(CreateOutput(0, DISPLAY_CONNECTION_TYPE_UNKNOWN, 2, 11)); + helper_delegate_->set_cached_outputs(outputs.get()); + + // External display should be updated if the id is zero. + DispatchOutputChangeEvent(2, 11, 20, true); + EXPECT_EQ(1, helper_delegate_->num_calls_notify_observers()); +} + } // namespace ui -- 2.11.4.GIT