Suppressed screen rotation notifications triggeres by the accelerometer

ash/system/chromeos/tray_display.cc

 // Copyright (c) 2012 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/system/chromeos/tray_display.h"
 
 #include "ash/display/display_controller.h"
 #include "ash/display/display_manager.h"
 #include "ash/shell.h"
 #include "ash/system/system_notifier.h"
 #include "ash/system/tray/actionable_view.h"
 #include "ash/system/tray/fixed_sized_image_view.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_delegate.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_notification_view.h"
+#include "ash/wm/maximize_mode/maximize_mode_controller.h"
 #include "base/bind.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/notification.h"
 #include "ui/message_center/notification_delegate.h"
@@ skipping 91 matching lines @@
     case user::LOGGED_IN_OWNER:
     case user::LOGGED_IN_GUEST:
     case user::LOGGED_IN_RETAIL_MODE:
     case user::LOGGED_IN_PUBLIC:
     case user::LOGGED_IN_LOCALLY_MANAGED:
     case user::LOGGED_IN_KIOSK_APP:
       Shell::GetInstance()->system_tray_delegate()->ShowDisplaySettings();
   }
 }
 
+// A message centre notification blocker used to suppress display notifications.
+// NOTE: This class should NOT be inherited because the destructor causes
+// observers to call back in to the instance being destroyed.
+class ScopedNotificationBlocker
+    : public message_center::NotificationBlocker {
+ public:
+  ScopedNotificationBlocker();
+  virtual ~ScopedNotificationBlocker();
+
+  // Overrides from message_center::NotificationBlocker:

[jonross, 2014/05/14 01:00:39]

nit: use the newer style

// message_center::NotificationBlocker:

[bruthig, 2014/05/14 18:48:49]

Done.

+  virtual bool ShouldShowNotificationAsPopup(
+      const message_center::NotifierId& notifier_id) const OVERRIDE;
+  virtual bool ShouldShowNotification(
+      const message_center::NotifierId& notifier_id) const OVERRIDE;
+
+ private:
+  // When true notifications should be shown
+  bool should_show_notifications_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedNotificationBlocker);
+};
+
+ScopedNotificationBlocker::ScopedNotificationBlocker()
+    : message_center::NotificationBlocker(
+      message_center::MessageCenter::Get()),
+      should_show_notifications_(false) {
+  NotifyBlockingStateChanged();
+}
+
+ScopedNotificationBlocker::~ScopedNotificationBlocker() {
+  should_show_notifications_ = true;
+  NotifyBlockingStateChanged();

[jonross, 2014/05/14 01:00:39]

Does this trigger calls to the subsequent methods?

If not there is no need for should_show_notifications.

[bruthig, 2014/05/14 18:48:49]

Yes it does.  NotifyBlockingStateChanged will cause
ScopedNotificationBlocker::ShouldShowNotificationAsPopup to be called.

+}
+
+bool ScopedNotificationBlocker::ShouldShowNotificationAsPopup(
+    const message_center::NotifierId& notifier_id) const {
+  if (notifier_id.id == system_notifier::kNotifierDisplay)
+    return should_show_notifications_;
+  return true;
+}
+
+bool ScopedNotificationBlocker::ShouldShowNotification(
+    const message_center::NotifierId& notifier_id) const {
+  if (notifier_id.id == system_notifier::kNotifierDisplay)
+    return should_show_notifications_;
+  return message_center::NotificationBlocker::ShouldShowNotification(
+      notifier_id);
+}
+
 }  // namespace
 
 const char TrayDisplay::kNotificationId[] = "chrome://settings/display";
 
 class DisplayView : public ActionableView {
  public:
   explicit DisplayView() {
     SetLayoutManager(new views::BoxLayout(
         views::BoxLayout::kHorizontal,
         kTrayPopupPaddingHorizontal, 0,
@@ skipping 243 matching lines @@
       message,
       additional_message,
       bundle.GetImageNamed(IDR_AURA_NOTIFICATION_DISPLAY),
       base::string16(),  // display_source
       message_center::NotifierId(
           message_center::NotifierId::SYSTEM_COMPONENT,
           system_notifier::kNotifierDisplay),
       message_center::RichNotificationData(),
       new message_center::HandleNotificationClickedDelegate(
           base::Bind(&OpenSettings))));
-  message_center::MessageCenter::Get()->AddNotification(notification.Pass());
+
+  message_center::MessageCenter* message_center =
+      message_center::MessageCenter::Get();
+  ash::MaximizeModeController* maximize_mode_controller =

[jonross, 2014/05/14 01:00:39]

nit: This is in the namespace of ash, don't need to specify it.

[bruthig, 2014/05/14 18:48:49]

Done.

+      ash::Shell::GetInstance()->maximize_mode_controller();
+  // Check to see if the display notification popups should be blocked.  This
+  // is used for things like suppressing redundant notifications for screen
+  // rotations caused by accelerometer events.
+  if (maximize_mode_controller &&
+      !maximize_mode_controller->show_display_notifications()) {
+    ScopedNotificationBlocker notification_blocker;
+    message_center->AddNotification(notification.Pass());
+    message_center->MarkSinglePopupAsShown(kNotificationId, false);

[jonross, 2014/05/14 01:00:39]

nit: comment on the meaning of false. In general for ambiguous cases like this
an enum is preferred. However I don't see it in the scope of this change to
update that method.

[flackr, 2014/05/14 01:48:33]

Not sure that we prefer enums for this on or off only two state kind of member.
The comment style is typically /* mark_notification_as_read */ false or the
other way around. Example:
https://code.google.com/p/chromium/codesearch#chromium/src/ui/message_center/...

That being said, can we just remove the current display notification if it's
there and not add any notifications when they're accelerometer triggered?

[bruthig, 2014/05/14 18:48:49]

Done.

+  } else {
+    message_center->AddNotification(notification.Pass());
+  }
 }
 
 views::View* TrayDisplay::CreateDefaultView(user::LoginStatus status) {
   DCHECK(default_ == NULL);
   default_ = new DisplayView();
   return default_;
 }
 
 void TrayDisplay::DestroyDefaultView() {
   default_ = NULL;
@@ skipping 27 matching lines @@
 bool TrayDisplay::GetAccessibleStateForTesting(ui::AXViewState* state) {
   views::View* view = default_;
   if (view) {
     view->GetAccessibleState(state);
     return true;
   }
   return false;
 }
 
 }  // namespace ash