Suppressed screen rotation notifications triggeres by the accelerometer

ash/wm/maximize_mode/maximize_mode_controller.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/wm/maximize_mode/maximize_mode_controller.h"
 
 #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 {
 
 namespace {
 
 // The hinge angle at which to enter maximize mode.
 const float kEnterMaximizeModeAngle = 200.0f;
 
 // The angle at which to exit maximize mode, this is specifically less than the
 // angle to enter maximize mode to prevent rapid toggling when near the angle.
@@ skipping 53 matching lines @@
   // shortest angle between |base| and |other| was counterclockwise with respect
   // to the surface represented by |normal| and this angle must be reversed.
   if (gfx::DotProduct(cross, normal) > 0.0f)
     angle = 360.0f - angle;
   return angle;
 }
 
 }  // 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);
 }
 
 MaximizeModeController::~MaximizeModeController() {
   Shell::GetInstance()->accelerometer_controller()->RemoveObserver(this);
 }
 
 void MaximizeModeController::OnAccelerometerUpdated(
     const gfx::Vector3dF& base,
     const gfx::Vector3dF& lid) {
   // Ignore the reading if it appears unstable. The reading is considered
   // unstable if it deviates too much from gravity and/or the magnitude of the
   // reading from the lid differs too much from the reading from the base.
   float base_magnitude = base.Length();
   float lid_magnitude = lid.Length();
   if (std::abs(base_magnitude - lid_magnitude) > kNoisyMagnitudeDeviation ||
       std::abs(base_magnitude - 1.0f) > kDeviationFromGravityThreshold ||
       std::abs(lid_magnitude - 1.0f) > kDeviationFromGravityThreshold) {
       return;
   }
 
   // Responding to the hinge rotation can change the maximize mode state which
   // affects screen rotation, so we handle hinge rotation first.
   HandleHingeRotation(base, lid);
   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);
   bool maximize_mode_engaged =
       Shell::GetInstance()->IsMaximizeModeWindowManagerEnabled();
   // Ignore the component of acceleration parallel to the hinge for the purposes
   // of hinge angle calculation.
   gfx::Vector3dF base_flattened(base);
   gfx::Vector3dF lid_flattened(lid);
   base_flattened.set_y(0.0f);
@@ skipping 79 matching lines @@
     new_rotation = gfx::Display::ROTATE_0;
   else if (angle < 180.0f)
     new_rotation = gfx::Display::ROTATE_270;
   else if (angle < 270.0f)
     new_rotation = gfx::Display::ROTATE_180;
 
   // When exiting maximize mode return rotation to 0. When entering, rotate to
   // 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
   // lock if the user changes the display settings while we are in maximize
   // mode. https://crbug.com/369505
   DisplayManager* display_manager = Shell::GetInstance()->display_manager();
   gfx::Display::Rotation current_rotation = display_manager->
       GetDisplayInfo(gfx::Display::InternalDisplayId()).rotation();
   rotation_locked_ =  current_rotation != gfx::Display::ROTATE_0;
   Shell::GetInstance()->EnableMaximizeModeWindowManager(true);
   event_blocker_.reset(new MaximizeModeEventBlocker);
 }
 
 void MaximizeModeController::LeaveMaximizeMode() {
   DisplayManager* display_manager = Shell::GetInstance()->display_manager();
   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();
 }
 
 }  // namespace ash