Suppressed screen rotation notifications triggeres by the accelerometer

ash/wm/maximize_mode/maximize_mode_controller.cc

Index: ash/wm/maximize_mode/maximize_mode_controller.cc
diff --git a/ash/wm/maximize_mode/maximize_mode_controller.cc b/ash/wm/maximize_mode/maximize_mode_controller.cc
index f438d4944b44d52792d28594d0ce0c858e44ec54..0a9fafcb8aa6bdf2428b26d5cebb2dcd455324da 100644
--- a/ash/wm/maximize_mode/maximize_mode_controller.cc
+++ b/ash/wm/maximize_mode/maximize_mode_controller.cc
@@ -7,8 +7,12 @@
 #include "ash/accelerometer/accelerometer_controller.h"
 #include "ash/display/display_manager.h"
 #include "ash/shell.h"
+#include "ash/system/chromeos/tray_display.h"
+#include "ash/system/system_notifier.h"
 #include "ash/wm/maximize_mode/maximize_mode_event_blocker.h"
 #include "ui/gfx/vector3d_f.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/notification_blocker.h"
 
 namespace ash {
 
@@ -82,7 +86,9 @@ float ClockwiseAngleBetweenVectorsInDegrees(const gfx::Vector3dF& base,
 }  // namespace
 
 MaximizeModeController::MaximizeModeController()
-    : rotation_locked_(false) {
+    : message_center::NotificationBlocker(message_center::MessageCenter::Get()),

[flackr, 2014/05/08 21:56:15]

Are there any potential lifetime issues here? Does the MessageCenter for sure
exist before and outlive this class?

[bruthig, 2014/05/14 00:36:16]

Done.

+      rotation_locked_(false),
+      show_notifications_(true) {
   Shell::GetInstance()->accelerometer_controller()->AddObserver(this);
 }
 
@@ -110,6 +116,28 @@ void MaximizeModeController::OnAccelerometerUpdated(
   HandleScreenRotation(lid);
 }
 
+void MaximizeModeController::set_show_notifications(bool should_show) {
+  bool did_change = show_notifications_ != should_show;
+  show_notifications_ = should_show;
+  if (did_change)
+    NotifyBlockingStateChanged();
+}
+
+bool MaximizeModeController::ShouldShowNotificationAsPopup(
+    const message_center::NotifierId& notifier_id) const {
+  if (notifier_id.id == ash::system_notifier::kNotifierDisplay)
+    return show_notifications_;
+  return true;

[jonross, 2014/05/09 14:57:33]

on 137 you fall back to checking with the message center. Is that also needed
here?

[bruthig, 2014/05/14 00:36:16]

For some reason ShouldShowNotificationAsPopup is a pure virtual function in the
parent class.

+}
+
+bool MaximizeModeController::ShouldShowNotification(
+    const message_center::NotifierId& notifier_id) const {
+  if (notifier_id.id == ash::system_notifier::kNotifierDisplay)
+    return show_notifications_;
+  return message_center::NotificationBlocker::ShouldShowNotification(
+      notifier_id);
+}
+
 void MaximizeModeController::HandleHingeRotation(const gfx::Vector3dF& base,
                                                  const gfx::Vector3dF& lid) {
   static const gfx::Vector3dF hinge_vector(0.0f, 1.0f, 0.0f);
@@ -209,11 +237,29 @@ void MaximizeModeController::HandleScreenRotation(const gfx::Vector3dF& lid) {
   // match screen orientation.
   if (new_rotation == gfx::Display::ROTATE_0 ||
       maximize_mode_engaged) {
-    display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
-                                        new_rotation);
+    SetDisplayRotation(display_manager,
+                       gfx::Display::InternalDisplayId(),
+                       new_rotation);
   }
 }
 
+// Sets the display's rotation.
+void MaximizeModeController::SetDisplayRotation(
+    DisplayManager* display_manager,
+    int64 display_id,
+    gfx::Display::Rotation rotation) {
+  // Suppress message centre notifications for screen rotations caused
+  // by accelerometer events because it should be obvious why the orientation
+  // changed.
+  set_show_notifications(false);

[flackr, 2014/05/08 21:56:15]

It still seems like it would be nicer to scope the NotificationBlocker to this
method. That being said I'll defer to sadrul.

[bruthig, 2014/05/14 00:36:16]

Done.

+  display_manager->SetDisplayRotation(display_id, rotation);
+  message_center::MessageCenter* message_center =
+      message_center::MessageCenter::Get();
+  if (message_center->HasNotification(TrayDisplay::kNotificationId))

[flackr, 2014/05/08 21:56:15]

This is a bit hacky, this could probably use a todo, perhaps the different
return values from notification blockers I suggested. i.e.
TODO(bruthig): Instead of inspecting the current notifications, we should use an
enum return from NotificationBlocker which allows permanently dismissing a
notification rather than only delaying it.

[bruthig, 2014/05/14 00:36:16]

Done.

+    message_center->MarkSinglePopupAsShown(TrayDisplay::kNotificationId, true);
+  set_show_notifications(true);
+}
+
 void MaximizeModeController::EnterMaximizeMode() {
   // TODO(jonross): Create a property on the display to track user rotations.
   // We should lock based on this property. Furthermore we should apply the
@@ -232,8 +278,9 @@ void MaximizeModeController::LeaveMaximizeMode() {
   gfx::Display::Rotation current_rotation = display_manager->
       GetDisplayInfo(gfx::Display::InternalDisplayId()).rotation();
   if (!rotation_locked_ && current_rotation != gfx::Display::ROTATE_0) {
-    display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
-                                        gfx::Display::ROTATE_0);
+    SetDisplayRotation(display_manager,
+                       gfx::Display::InternalDisplayId(),
+                       gfx::Display::ROTATE_0);
   }
   Shell::GetInstance()->EnableMaximizeModeWindowManager(false);
   event_blocker_.reset();