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}

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 8e61b9b..9e206ac 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<const DisplayModeX11*>(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 de87796..eb125f3 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<int64_t>(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<DisplaySnapshot> 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<DisplaySnapshot> 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<DisplaySnapshot> 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<DisplaySnapshot> 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<DisplaySnapshot> 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<DisplaySnapshot> 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<DisplaySnapshot> 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<DisplaySnapshot> 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<DisplaySnapshot> 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