Reland 2 of r279550 "Allow notification updates on ChromeOS."

This fixes a regression with multi-profile mode where updated
notifications don't properly have the profile_id set.

Original patches were reverted due to memory leaks in the tests.

BUG=386814

Review URL: https://codereview.chromium.org/358473007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@280127 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 111 insertions, 37 deletions

diff --git a/chrome/browser/notifications/message_center_notification_manager.cc b/chrome/browser/notifications/message_center_notification_manager.cc
index f400bfd..a0ae9d19 100644
--- a/chrome/browser/notifications/message_center_notification_manager.cc
+++ b/chrome/browser/notifications/message_center_notification_manager.cc
@@ -8,6 +8,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
+#include "base/stl_util.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/notifications/desktop_notification_service_factory.h"
@@ -91,6 +92,10 @@ MessageCenterNotificationManager::MessageCenterNotificationManager(
 MessageCenterNotificationManager::~MessageCenterNotificationManager() {
   message_center_->SetNotifierSettingsProvider(NULL);
   message_center_->RemoveObserver(this);
+
+  STLDeleteContainerPairSecondPointers(profile_notifications_.begin(),
+                                       profile_notifications_.end());
+  profile_notifications_.clear();
 }
 
 void MessageCenterNotificationManager::RegisterPrefs(
@@ -112,6 +117,9 @@ void MessageCenterNotificationManager::Add(const Notification& notification,
   DesktopNotificationServiceFactory::GetForProfile(profile)->
       ShowWelcomeNotificationIfNecessary(notification);
 
+  // WARNING: You MUST use AddProfileNotification or update the message center
+  // via the notification within a ProfileNotification object or the profile ID
+  // will not be correctly set for ChromeOS.
   AddProfileNotification(
       new ProfileNotification(profile, notification, message_center_));
 }
@@ -138,7 +146,6 @@ bool MessageCenterNotificationManager::Update(const Notification& notification,
       // the immediate update allowed in the message center.
       std::string old_id =
           old_notification->notification().delegate_id();
-      DCHECK(message_center_->FindVisibleNotificationById(old_id));
 
       // Add/remove notification in the local list but just update the same
       // one in MessageCenter.
@@ -148,11 +155,13 @@ bool MessageCenterNotificationManager::Update(const Notification& notification,
           new ProfileNotification(profile, notification, message_center_);
       profile_notifications_[notification.delegate_id()] = new_notification;
 
-      // Now pass a copy to message center.
-      scoped_ptr<message_center::Notification> message_center_notification(
-          make_scoped_ptr(new message_center::Notification(notification)));
-      message_center_->UpdateNotification(old_id,
-                                          message_center_notification.Pass());
+      // WARNING: You MUST use AddProfileNotification or update the message
+      // center via the notification within a ProfileNotification object or the
+      // profile ID will not be correctly set for ChromeOS.
+      message_center_->UpdateNotification(
+          old_id,
+          make_scoped_ptr(new message_center::Notification(
+              new_notification->notification())));
 
       new_notification->StartDownloads();
       return true;
diff --git a/chrome/browser/notifications/message_center_notifications_unittest_win.cc b/chrome/browser/notifications/message_center_notifications_unittest.cc
index 39f229f..058881d 100644
--- a/chrome/browser/notifications/message_center_notifications_unittest_win.cc
+++ b/chrome/browser/notifications/message_center_notifications_unittest.cc
@@ -5,15 +5,18 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/testing_pref_service.h"
 #include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/test/test_timeouts.h"
 #include "base/values.h"
 #include "chrome/browser/notifications/message_center_notification_manager.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_test_util.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/scoped_testing_local_state.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/message_center/fake_message_center_tray_delegate.h"
@@ -23,45 +26,72 @@
 #include "ui/message_center/message_center_types.h"
 #include "ui/message_center/notifier_settings.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/ui/ash/multi_user/multi_user_notification_blocker_chromeos.h"
+#include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
+#include "chrome/browser/ui/ash/multi_user/multi_user_window_manager.h"
+#include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.h"
+#endif
+
 namespace message_center {
 
-class MessageCenterNotificationManagerTest : public testing::Test {
- protected:
-  MessageCenterNotificationManagerTest() {
-    MessageCenterNotificationManager::RegisterPrefs(local_state_.registry());
-  }
+class MessageCenterNotificationManagerTest : public BrowserWithTestWindowTest {
+ public:
+  MessageCenterNotificationManagerTest() {}
 
+ protected:
   virtual void SetUp() {
+    BrowserWithTestWindowTest::SetUp();
+#if !defined(OS_CHROMEOS)
+    // BrowserWithTestWindowTest owns an AshTestHelper on OS_CHROMEOS, which
+    // in turn initializes the message center.  On other platforms, we need to
+    // initialize it here.
+    MessageCenter::Initialize();
+#endif
+
+
     // Clear the preference and initialize.
-    local_state_.ClearPref(prefs::kMessageCenterShowedFirstRunBalloon);
-    first_run_pref_.Init(prefs::kMessageCenterShowedFirstRunBalloon,
-                         &local_state_);
+    TestingBrowserProcess* browser_process = TestingBrowserProcess::GetGlobal();
+    profile_manager_.reset(new TestingProfileManager(browser_process));
+    ASSERT_TRUE(profile_manager_->SetUp());
+    local_state()->ClearPref(prefs::kMessageCenterShowedFirstRunBalloon);
+    first_run_pref_.reset(new BooleanPrefMember);
+    first_run_pref_->Init(prefs::kMessageCenterShowedFirstRunBalloon,
+                         local_state());
 
     // Get ourselves a run loop.
     run_loop_.reset(new base::RunLoop());
 
-    // Initialize message center infrastructure with mock tray delegate.
-    MessageCenter::Initialize();
     message_center_ = MessageCenter::Get();
     scoped_ptr<NotifierSettingsProvider> settings_provider(
         new FakeNotifierSettingsProvider(notifiers_));
-    notification_manager_.reset(new MessageCenterNotificationManager(
-        message_center_, &local_state_, settings_provider.Pass()));
     delegate_ = new FakeMessageCenterTrayDelegate(message_center_,
                                                   run_loop_->QuitClosure());
-    notification_manager_->SetMessageCenterTrayDelegateForTest(delegate_);
-    notification_manager_->SetFirstRunTimeoutForTest(
+    notification_manager()->SetMessageCenterTrayDelegateForTest(delegate_);
+#if defined(OS_WIN)
+    // First run features are only implemented on Windows, where the
+    // notification center is hard to find.
+    notification_manager()->SetFirstRunTimeoutForTest(
         TestTimeouts::tiny_timeout());
+#endif
   }
 
   virtual void TearDown() {
     run_loop_.reset();
-    notification_manager_.reset();
+    first_run_pref_.reset();
+    profile_manager_.reset();
+
+#if !defined(OS_CHROMEOS)
+    // Shutdown the message center if we initialized it manually.
     MessageCenter::Shutdown();
+#endif
+
+    BrowserWithTestWindowTest::TearDown();
   }
 
   MessageCenterNotificationManager* notification_manager() {
-    return notification_manager_.get();
+    return (MessageCenterNotificationManager*)
+        g_browser_process->notification_ui_manager();
   }
 
   FakeMessageCenterTrayDelegate* delegate() { return delegate_; }
@@ -69,29 +99,30 @@ class MessageCenterNotificationManagerTest : public testing::Test {
   MessageCenter* message_center() { return message_center_; }
 
   const ::Notification GetANotification(const std::string& id) {
-    return ::Notification(GURL(),
-                          GURL(),
-                          base::string16(),
-                          base::string16(),
-                          blink::WebTextDirectionDefault,
-                          base::string16(),
-                          base::string16(),
-                          new MockNotificationDelegate(id));
+    return ::Notification(
+        GURL("chrome-extension://adflkjsdflkdsfdsflkjdsflkdjfs"),
+        GURL(),
+        base::string16(),
+        base::string16(),
+        blink::WebTextDirectionDefault,
+        base::string16(),
+        base::UTF8ToUTF16(id),
+        new MockNotificationDelegate(id));
   }
 
   base::RunLoop* run_loop() { return run_loop_.get(); }
-  const TestingPrefServiceSimple& local_state() { return local_state_; }
-  bool DidFirstRunPref() { return first_run_pref_.GetValue(); }
+  PrefService* local_state() {
+    return TestingBrowserProcess::GetGlobal()->local_state();
+  }
+  bool DidFirstRunPref() { return first_run_pref_->GetValue(); }
 
  private:
+  scoped_ptr<TestingProfileManager> profile_manager_;
   scoped_ptr<base::RunLoop> run_loop_;
-  TestingPrefServiceSimple local_state_;
   MessageCenter* message_center_;
   std::vector<Notifier*> notifiers_;
-  scoped_ptr<MessageCenterNotificationManager> notification_manager_;
   FakeMessageCenterTrayDelegate* delegate_;
-  content::TestBrowserThreadBundle thread_bundle_;
-  BooleanPrefMember first_run_pref_;
+  scoped_ptr<BooleanPrefMember> first_run_pref_;
 };
 
 TEST_F(MessageCenterNotificationManagerTest, SetupNotificationManager) {
@@ -100,6 +131,38 @@ TEST_F(MessageCenterNotificationManagerTest, SetupNotificationManager) {
   EXPECT_FALSE(DidFirstRunPref());
 }
 
+TEST_F(MessageCenterNotificationManagerTest, UpdateNotification) {
+  TestingProfile profile;
+  EXPECT_TRUE(message_center()->NotificationCount() == 0);
+  notification_manager()->Add(GetANotification("test"), &profile);
+  EXPECT_TRUE(message_center()->NotificationCount() == 1);
+  ASSERT_TRUE(
+      notification_manager()->Update(GetANotification("test"), &profile));
+  EXPECT_TRUE(message_center()->NotificationCount() == 1);
+}
+
+#if defined(OS_CHROMEOS)
+TEST_F(MessageCenterNotificationManagerTest, MultiUserUpdates) {
+  TestingProfile profile;
+  std::string active_user_id = multi_user_util::GetUserIDFromProfile(&profile);
+  chrome::MultiUserWindowManagerChromeOS* multi_user_window_manager =
+      new chrome::MultiUserWindowManagerChromeOS(active_user_id);
+  chrome::MultiUserWindowManager::SetInstanceForTest(
+      multi_user_window_manager,
+      chrome::MultiUserWindowManager::MULTI_PROFILE_MODE_SEPARATED);
+  scoped_ptr<MultiUserNotificationBlockerChromeOS> blocker(
+      new MultiUserNotificationBlockerChromeOS(
+          message_center::MessageCenter::Get(),
+          active_user_id));
+  EXPECT_EQ(0u, message_center()->NotificationCount());
+  notification_manager()->Add(GetANotification("test"), &profile);
+  EXPECT_EQ(1u, message_center()->NotificationCount());
+  notification_manager()->Update(GetANotification("test"), &profile);
+  EXPECT_EQ(1u, message_center()->NotificationCount());
+}
+#endif
+
+#if defined(OS_WIN)
 // The following tests test the first run balloon, which is only implemented for
 // Windows.
 TEST_F(MessageCenterNotificationManagerTest, FirstRunShown) {
@@ -137,4 +200,6 @@ TEST_F(MessageCenterNotificationManagerTest,
   EXPECT_FALSE(notification_manager()->FirstRunTimerIsActive());
   EXPECT_FALSE(DidFirstRunPref());
 }
+
+#endif
 }  // namespace message_center
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 7dcd69c..40c3e0e 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -1138,7 +1138,7 @@
         'browser/notifications/desktop_notification_service_unittest.cc',
         'browser/notifications/extension_welcome_notification_unittest.cc',
         'browser/notifications/login_state_notification_blocker_chromeos_unittest.cc',
-        'browser/notifications/message_center_notifications_unittest_win.cc',
+        'browser/notifications/message_center_notifications_unittest.cc',
         'browser/notifications/message_center_settings_controller_unittest.cc',
         'browser/notifications/sync_notifier/chrome_notifier_delegate_unittest.cc',
         'browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc',