Corrected mirroring updates in ime_controller updates

There was a misunderstanding in how OnDisplayMetricChanged's
changed_metrics worked. The implementation in ImeController
assumed that the DISPLAM_METRIC_MIRROR_STATE will be true when mirroring
is occuring.

What actually happens is that OnDisplayMetricChaniged will have it's
changed_metrics bitflag set to true when the metric has changed value
(from true to false or false to true)

The implementation has been updated to reflect that and the function
documentation was updated to reflect that more clearly.

Change-Id: I110b5b1fbee11f1c761b2c5d715b7034d8c44cd6
Bug: 824656
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1600616
Commit-Queue: Keith Lee <keithlee@google.com>
Auto-Submit: Keith Lee <keithlee@google.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662872}

ash/ime/ime_controller.cc

@@ -196,9 +196,10 @@ void ImeController::ShowModeIndicator(const gfx::Rect& anchor_bounds,
 
 void ImeController::OnDisplayMetricsChanged(const display::Display& display,
                                             uint32_t changed_metrics) {
-  bool is_mirroring =
-      (changed_metrics & display::DisplayObserver::DISPLAY_METRIC_MIRROR_STATE);
-  client_->UpdateMirroringState(is_mirroring);
+  if (changed_metrics & display::DisplayObserver::DISPLAY_METRIC_MIRROR_STATE) {
+    Shell* shell = Shell::Get();
+    client_->UpdateMirroringState(shell->display_manager()->IsInMirrorMode());
+  }
 }
 
 void ImeController::OnDevicesUpdated(const std::vector<SinkAndRoute>& devices) {

ash/ime/ime_controller_unittest.cc

@@ -16,6 +16,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/ime/chromeos/extension_ime_util.h"
+#include "ui/display/manager/display_manager.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/views/widget/widget.h"
 
@@ -353,5 +354,27 @@ TEST_F(ImeControllerTest, ShowModeIndicator) {
   EXPECT_LT(bounds3.bottom(), screen_bounds.bottom());
 }
 
+TEST_F(ImeControllerTest, MirroringChanged) {
+  UpdateDisplay("500x500,500x500");
+  // The controller is already an observer of the display_manager
+  ImeController* controller = Shell::Get()->ime_controller();
+  TestImeControllerClient client;
+  controller->SetClient(client.CreateInterfacePtr());
+
+  display::DisplayManager* display_manager = Shell::Get()->display_manager();
+  display_manager->SetMultiDisplayMode(display::DisplayManager::MIRRORING);
+  display_manager->UpdateDisplays();
+  controller->FlushMojoForTesting();
+  EXPECT_TRUE(client.is_mirroring_);
+
+  UpdateDisplay("500x500");
+  controller->FlushMojoForTesting();
+  EXPECT_FALSE(client.is_mirroring_);
+
+  UpdateDisplay("500x500,500x500");
+  controller->FlushMojoForTesting();
+  EXPECT_TRUE(client.is_mirroring_);
+}
+
 }  // namespace
 }  // namespace ash

ui/display/display_observer.h

@@ -41,8 +41,11 @@ class DISPLAY_EXPORT DisplayObserver : public base::CheckedObserver {
   // Called when |old_display| has been removed.
   virtual void OnDisplayRemoved(const Display& old_display);
 
-  // Called when a |display| has one or more metrics changed. |changed_metrics|
-  // will contain the information about the change, see |DisplayMetric|.
+  // Called when the metrics of a display change.
+  // |changed_metrics| is a bitmask of DisplayMatric types indicating which
+  // metrics have changed. Eg; if mirroring changes (either from true to false,
+  // or false to true), than the DISPLAY_METRIC_MIRROR_STATE bit is set in
+  // changed_metrics.
   virtual void OnDisplayMetricsChanged(const Display& display,
                                        uint32_t changed_metrics);