Issue 20066003: Update notification settings to allow for multiprofile situations.

Issue 20066003: Update notification settings to allow for multiprofile situations. (Closed)

Created:
7 years, 5 months ago by dewittj

Modified:
7 years, 4 months ago

Reviewers:
sky, jianli

CC:
chromium-reviews

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Update notification settings to allow for multiprofile situations. This introduces the NotifierGroup concept to ui/message_center, which will correspond directly to Profiles. The user will be able to select a NotifierGroup and then update the notification settings relevant to that group/profile. TBR=sky@chromium.org R=jianli@chromium.org BUG=254511, 256415, 218402 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216186

Patch Set 1 : Updated with newest mock. #

Patch Set 2 : Rebase. #

Patch Set 3 : Fix linux compile #

Patch Set 4 : Fix compile on linux_aura/chromeos #

Patch Set 5 : Add unittest. #

Patch Set 6 : Fix message_center_unittests #

Total comments: 34

Patch Set 7 : Address jianli comments #

Total comments: 1

Patch Set 8 : Leave string alone on Mac until new interface is implemented. #

Patch Set 9 : Rebase. #

Patch Set 10 : Fix chromeos screenshot test. #

Patch Set 11 : And remove unneeded include. #

Created: 7 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+887 lines, -365 lines)			Patch
M	chrome/browser/notifications/message_center_notification_manager.h	View	1 2 3 4 5 6	2 chunks	+4 lines, -3 lines	0 comments	Download
M	chrome/browser/notifications/message_center_notification_manager.cc	View	1 2 3 4 5 6	1 chunk	+4 lines, -3 lines	0 comments	Download
M	chrome/browser/notifications/message_center_notifications_unittest_win.cc	View		3 chunks	+7 lines, -2 lines	0 comments	Download
M	chrome/browser/notifications/message_center_settings_controller.h	View	1 2 3 4 5 6 7 8 9	4 chunks	+29 lines, -13 lines	0 comments	Download
M	chrome/browser/notifications/message_center_settings_controller.cc	View	1 2 3 4 5 6	14 chunks	+130 lines, -39 lines	0 comments	Download
A	chrome/browser/notifications/message_center_settings_controller_unittest.cc	View	1 2 3 4 5 6	1 chunk	+71 lines, -0 lines	0 comments	Download
M	chrome/browser/notifications/notification_ui_manager.cc	View	1 2 3 4	2 chunks	+13 lines, -2 lines	0 comments	Download
M	chrome/browser/notifications/notification_ui_manager_mac.mm	View	1 2 3 4 5 6	2 chunks	+11 lines, -1 line	0 comments	Download
M	chrome/browser/ui/ash/screenshot_taker_unittest.cc	View	1 2 3 4 5 6 7 8 9 10	4 chunks	+7 lines, -7 lines	0 comments	Download
M	chrome/chrome_tests_unit.gypi	View	1 2 3 4 5 6 7 8	2 chunks	+2 lines, -0 lines	0 comments	Download
M	ui/base/strings/ui_strings.grd	View	1 2 3 4 5 6 7	1 chunk	+10 lines, -3 lines	0 comments	Download
M	ui/message_center/cocoa/settings_controller.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	ui/message_center/cocoa/settings_controller.mm	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	ui/message_center/fake_notifier_settings_provider.h	View	1 2 3 4 5 6	2 chunks	+9 lines, -1 line	0 comments	Download
M	ui/message_center/fake_notifier_settings_provider.cc	View	1 2 3 4 5 6	1 chunk	+23 lines, -1 line	0 comments	Download
M	ui/message_center/message_center.gyp	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	ui/message_center/notifier_settings.h	View	1 2 3 4 5 6	2 chunks	+44 lines, -0 lines	0 comments	Download
M	ui/message_center/notifier_settings.cc	View	1 2 3 4	1 chunk	+9 lines, -2 lines	0 comments	Download
A	ui/message_center/views/message_center_button_bar.h	View	1 2 3 4 5 6	1 chunk	+68 lines, -0 lines	0 comments	Download
A	ui/message_center/views/message_center_button_bar.cc	View	1 2 3 4 5 6	1 chunk	+210 lines, -0 lines	0 comments	Download
M	ui/message_center/views/message_center_view.h	View	1 2 3 4 5 6 7 8	3 chunks	+12 lines, -3 lines	0 comments	Download
M	ui/message_center/views/message_center_view.cc	View	1 2 3 4 5 6 7 8	9 chunks	+50 lines, -263 lines	0 comments	Download
M	ui/message_center/views/notifier_settings_view.h	View	1 2 3 4 5 6	4 chunks	+19 lines, -1 line	0 comments	Download
M	ui/message_center/views/notifier_settings_view.cc	View	1 2 3 4 5 6	6 chunks	+150 lines, -21 lines	0 comments	Download

Messages

Total messages: 16 (0 generated)

Expand Messages | Collapse Messages

jianli

https://codereview.chromium.org/20066003/diff/73001/chrome/browser/notifications/message_center_notification_manager.h File chrome/browser/notifications/message_center_notification_manager.h (right): https://codereview.chromium.org/20066003/diff/73001/chrome/browser/notifications/message_center_notification_manager.h#newcode216 chrome/browser/notifications/message_center_notification_manager.h:216: scoped_ptr<message_center::NotifierSettingsProvider> settings_controller_; Should we rename settings_controller_ to settings_provider_? https://codereview.chromium.org/20066003/diff/73001/chrome/browser/notifications/message_center_settings_controller.cc ...

7 years, 4 months ago (2013-08-05 20:48:03 UTC) #2

dewittj

7 years, 4 months ago (2013-08-05 22:42:28 UTC) #3

jianli

https://codereview.chromium.org/20066003/diff/87001/ui/message_center/views/notifier_settings_view.cc File ui/message_center/views/notifier_settings_view.cc (right): https://codereview.chromium.org/20066003/diff/87001/ui/message_center/views/notifier_settings_view.cc#newcode7 ui/message_center/views/notifier_settings_view.cc:7: #include <set> nit: this can be removed now.

7 years, 4 months ago (2013-08-05 23:07:26 UTC) #4

dewittj

TBR sky=ui/base/strings/ui_strings.grd Since Mac is a TODO for the UI, latest change leaves the string ...

7 years, 4 months ago (2013-08-05 23:38:13 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dewittj@chromium.org/20066003/126001

7 years, 4 months ago (2013-08-06 00:01:52 UTC) #7

commit-bot: I haz the power

Failed to apply patch for ui/message_center/views/message_center_view.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...

7 years, 4 months ago (2013-08-06 00:02:12 UTC) #8

Failed to apply patch for ui/message_center/views/message_center_view.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/message_center/views/message_center_view.cc
  Hunk #3 FAILED at 592.
  Hunk #4 succeeded at 619 (offset 1 line).
  Hunk #5 succeeded at 709 with fuzz 2 (offset 16 lines).
  Hunk #6 FAILED at 716.
  Hunk #7 succeeded at 758 (offset 22 lines).
  Hunk #8 succeeded at 801 (offset 22 lines).
  Hunk #9 succeeded at 950 (offset 25 lines).
  2 out of 9 hunks FAILED -- saving rejects to file
ui/message_center/views/message_center_view.cc.rej

Patch:       ui/message_center/views/message_center_view.cc
Index: ui/message_center/views/message_center_view.cc
diff --git a/ui/message_center/views/message_center_view.cc
b/ui/message_center/views/message_center_view.cc
index
0792ec8a690f0d16e98e9538f967a3b36071f008..b13ada6b77445237b24caea02f3139ca4f1a7a5a
100644
--- a/ui/message_center/views/message_center_view.cc
+++ b/ui/message_center/views/message_center_view.cc
@@ -10,22 +10,17 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
-#include "grit/ui_resources.h"
 #include "grit/ui_strings.h"
 #include "ui/base/animation/multi_animation.h"
 #include "ui/base/animation/slide_animation.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/insets.h"
-#include "ui/gfx/point.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size.h"
-#include "ui/gfx/text_constants.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/message_center_style.h"
-#include "ui/message_center/message_center_tray.h"
-#include "ui/message_center/message_center_util.h"
+#include "ui/message_center/views/message_center_button_bar.h"
 #include "ui/message_center/views/message_view.h"
 #include "ui/message_center/views/notification_view.h"
 #include "ui/message_center/views/notifier_settings_view.h"
@@ -34,238 +29,30 @@
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/button.h"
-#include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/controls/scrollbar/overlay_scroll_bar.h"
 #include "ui/views/layout/box_layout.h"
-#include "ui/views/layout/grid_layout.h"
-#include "ui/views/painter.h"
 #include "ui/views/widget/widget.h"
 
 namespace message_center {
 
 namespace {
 
-const int kMinScrollViewHeight = 100;
-const int kFooterLeftMargin = 17;
-const int kFooterRightMargin = 14;
-const int kButtonSize = 40;
-const SkColor kNoNotificationsTextColor = SkColorSetRGB(0xb4, 0xb4, 0xb4);
 const SkColor kBorderDarkColor = SkColorSetRGB(0xaa, 0xaa, 0xaa);
-const SkColor kTransparentColor = SkColorSetARGB(0, 0, 0, 0);
 const SkColor kButtonTextHighlightColor = SkColorSetRGB(0x2a, 0x2a, 0x2a);
 const SkColor kButtonTextHoverColor = SkColorSetRGB(0x2a, 0x2a, 0x2a);
+const SkColor kNoNotificationsTextColor = SkColorSetRGB(0xb4, 0xb4, 0xb4);
+const SkColor kTransparentColor = SkColorSetARGB(0, 0, 0, 0);
 const int kAnimateClearingNextNotificationDelayMS = 40;
+const int kButtonBarBorderThickness = 1;
+const int kMinScrollViewHeight = 100;
 
-static const int kDefaultFrameRateHz = 60;
 static const int kDefaultAnimationDurationMs = 120;
+static const int kDefaultFrameRateHz = 60;
 
 }  // namespace
 
-// NotificationCenterButton
////////////////////////////////////////////////////
-
-class NotificationCenterButton : public views::ToggleImageButton {
- public:
-  NotificationCenterButton(views::ButtonListener* listener,
-                           int normal_id,
-                           int hover_id,
-                           int pressed_id,
-                           int text_id);
-
- protected:
-  // Overridden from views::View:
-  virtual gfx::Size GetPreferredSize() OVERRIDE;
-  virtual void OnPaintFocusBorder(gfx::Canvas* canvas) OVERRIDE;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(NotificationCenterButton);
-};
-
-NotificationCenterButton::NotificationCenterButton(
-    views::ButtonListener* listener,
-    int normal_id,
-    int hover_id,
-    int pressed_id,
-    int text_id)
-    : views::ToggleImageButton(listener) {
-  ui::ResourceBundle& resource_bundle =
ui::ResourceBundle::GetSharedInstance();
-  SetImage(STATE_NORMAL, resource_bundle.GetImageSkiaNamed(normal_id));
-  SetImage(STATE_HOVERED, resource_bundle.GetImageSkiaNamed(hover_id));
-  SetImage(STATE_PRESSED, resource_bundle.GetImageSkiaNamed(pressed_id));
-  SetImageAlignment(views::ImageButton::ALIGN_CENTER,
-                    views::ImageButton::ALIGN_MIDDLE);
-  SetTooltipText(resource_bundle.GetLocalizedString(text_id));
-  set_focusable(true);
-  set_request_focus_on_press(false);
-}
-
-gfx::Size NotificationCenterButton::GetPreferredSize() {
-  return gfx::Size(kButtonSize, kButtonSize);
-}
-
-void NotificationCenterButton::OnPaintFocusBorder(gfx::Canvas* canvas) {
-  if (HasFocus() && (focusable() || IsAccessibilityFocusable())) {
-    canvas->DrawRect(gfx::Rect(2, 1, width() - 4, height() - 3),
-                     kFocusBorderColor);
-  }
-}
-
-// MessageCenterButtonBar //////////////////////////////////////////////////
-
-class MessageCenterButtonBar : public views::View,
-                               public views::ButtonListener {
- public:
-  MessageCenterButtonBar(MessageCenterView* message_center_view,
-                         MessageCenter* message_center);
-  virtual ~MessageCenterButtonBar();
-
-  virtual void SetAllButtonsEnabled(bool enabled);
-  void SetCloseAllVisible(bool visible);
-
- private:
-  // Overridden from views::View:
-  virtual void ChildVisibilityChanged(views::View* child) OVERRIDE;
-
-  // Overridden from views::ButtonListener:
-  virtual void ButtonPressed(views::Button* sender, const ui::Event& event)
-      OVERRIDE;
-
-  MessageCenterView* message_center_view() const {
-    return message_center_view_;
-  }
-  MessageCenter* message_center() const { return message_center_; }
-  MessageCenterTray* tray() const { return tray_; }
-  views::Button* close_all_button() const { return close_all_button_; }
-  void set_close_all_button(views::Button* button) {
-    close_all_button_ = button;
-  }
-
-  MessageCenterView* message_center_view_;  // Weak reference.
-  MessageCenter* message_center_;  // Weak reference.
-  MessageCenterTray* tray_;  // Weak reference.
-  views::Button* close_all_button_;
-  NotificationCenterButton* settings_button_;
-  NotificationCenterButton* quiet_mode_button_;
-
-  DISALLOW_COPY_AND_ASSIGN(MessageCenterButtonBar);
-};
-
-MessageCenterButtonBar::MessageCenterButtonBar(
-    MessageCenterView* message_center_view,
-    MessageCenter* message_center)
-    : message_center_view_(message_center_view),
-      message_center_(message_center),
-      close_all_button_(NULL) {
-  if (get_use_acceleration_when_possible())
-    SetPaintToLayer(true);
-  set_background(views::Background::CreateSolidBackground(
-      kMessageCenterBackgroundColor));
-
-  views::Label* notification_label = new
views::Label(l10n_util::GetStringUTF16(
-      IDS_MESSAGE_CENTER_FOOTER_TITLE));
-  notification_label->SetAutoColorReadabilityEnabled(false);
-  notification_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  notification_label->SetEnabledColor(kRegularTextColor);
-  AddChildView(notification_label);
-
-  views::View* button_container = new views::View;
-  button_container->SetLayoutManager(
-      new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0));
-  quiet_mode_button_ = new NotificationCenterButton(
-      this,
-      IDR_NOTIFICATION_DO_NOT_DISTURB,
-      IDR_NOTIFICATION_DO_NOT_DISTURB_HOVER,
-      IDR_NOTIFICATION_DO_NOT_DISTURB_PRESSED,
-      IDS_MESSAGE_CENTER_QUIET_MODE_BUTTON_TOOLTIP);
-  ui::ResourceBundle& resource_bundle =
ui::ResourceBundle::GetSharedInstance();
-  quiet_mode_button_->SetToggledImage(
-      views::Button::STATE_NORMAL,
-      resource_bundle.GetImageSkiaNamed(
-          IDR_NOTIFICATION_DO_NOT_DISTURB_PRESSED));
-  quiet_mode_button_->SetToggledImage(
-      views::Button::STATE_HOVERED,
-      resource_bundle.GetImageSkiaNamed(
-          IDR_NOTIFICATION_DO_NOT_DISTURB_PRESSED));
-  quiet_mode_button_->SetToggledImage(
-      views::Button::STATE_PRESSED,
-      resource_bundle.GetImageSkiaNamed(
-          IDR_NOTIFICATION_DO_NOT_DISTURB_PRESSED));
-  quiet_mode_button_->SetToggled(message_center->IsQuietMode());
-  button_container->AddChildView(quiet_mode_button_);
-
-  NotificationCenterButton* close_all_button = new NotificationCenterButton(
-      this,
-      IDR_NOTIFICATION_CLEAR_ALL,
-      IDR_NOTIFICATION_CLEAR_ALL_HOVER,
-      IDR_NOTIFICATION_CLEAR_ALL_PRESSED,
-      IDS_MESSAGE_CENTER_CLEAR_ALL);
-  button_container->AddChildView(close_all_button);
-  set_close_all_button(close_all_button);
-  settings_button_ = new NotificationCenterButton(
-      this,
-      IDR_NOTIFICATION_SETTINGS,
-      IDR_NOTIFICATION_SETTINGS_HOVER,
-      IDR_NOTIFICATION_SETTINGS_PRESSED,
-      IDS_MESSAGE_CENTER_SETTINGS_BUTTON_LABEL);
-  button_container->AddChildView(settings_button_);
-
-  gfx::ImageSkia* settings_image =
-      ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
-          IDR_NOTIFICATION_SETTINGS);
-  int image_margin = std::max(0, (kButtonSize - settings_image->width()) / 2);
-  views::GridLayout* layout = new views::GridLayout(this);
-  SetLayoutManager(layout);
-  layout->SetInsets(
-      0, kFooterLeftMargin, 0, std::max(0, kFooterRightMargin - image_margin));
-  views::ColumnSet* column = layout->AddColumnSet(0);
-  column->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
-                    1.0f, views::GridLayout::USE_PREF, 0, 0);
-  column->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL,
-                    0, views::GridLayout::USE_PREF, 0, 0);
-  layout->…
(message too large)

sky

Note, I made ui/base/strings OWNERS=*, so you shouldn't need to TBR me in the future. ...

7 years, 4 months ago (2013-08-06 00:04:56 UTC) #9

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dewittj@chromium.org/20066003/132001

7 years, 4 months ago (2013-08-06 00:36:38 UTC) #10

commit-bot: I haz the power

Retried try job too often on linux_chromeos for step(s) unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=142487

7 years, 4 months ago (2013-08-06 02:03:06 UTC) #11

dewittj

Uploaded a new patch which changes screenshot_taker_unittest.cc - TBR sky again.

7 years, 4 months ago (2013-08-06 21:54:54 UTC) #12

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dewittj@chromium.org/20066003/151001

7 years, 4 months ago (2013-08-06 21:59:41 UTC) #13

commit-bot: I haz the power

Retried try job too often on win_rel for step(s) telemetry_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=184334

7 years, 4 months ago (2013-08-07 03:10:16 UTC) #14

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dewittj@chromium.org/20066003/151001

7 years, 4 months ago (2013-08-07 14:28:33 UTC) #15

Message was sent while issue was closed.

Change committed as 216186

Expand Messages | Collapse Messages