Close privet printer notifications when clicked.

chrome/browser/printing/cloud_print/privet_notifications.cc

 // Copyright 2013 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 "chrome/browser/printing/cloud_print/privet_notifications.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 237 matching lines @@
       message_center::ButtonInfo(l10n_util::GetStringUTF16(
           IDS_LOCAL_DISCOVERY_NOTIFICATIONS_DISABLE_BUTTON_LABEL)));
 
   base::string16 title = l10n_util::GetPluralStringFUTF16(
       IDS_LOCAL_DISCOVERY_NOTIFICATION_TITLE_PRINTER, devices_active);
   base::string16 body = l10n_util::GetPluralStringFUTF16(
       IDS_LOCAL_DISCOVERY_NOTIFICATION_CONTENTS_PRINTER, devices_active);
   base::string16 product_name =
       l10n_util::GetStringUTF16(IDS_LOCAL_DISCOVERY_SERVICE_NAME_PRINTER);
 
+  Profile* profile = Profile::FromBrowserContext(profile_);

[yoshiki, 2016/10/26 16:24:05]

I think you don't need to convert the profile.
Profile::FromBrowserContext(Profile) is just a static casting.

[Lei Zhang, 2016/10/26 16:52:41]

Profile is a BrowserContext, but a BrowserContext is not a Profile, so I cannot
pass it to the PrivetNotificationDelegate ctor as a BrowserContext. I will work
on storing a Profile* in a separate CL, to reduce the number of
Profile::FromBrowserContext() calls.

   Notification notification(
       message_center::NOTIFICATION_TYPE_SIMPLE, title, body,
       ui::ResourceBundle::GetSharedInstance().GetImageNamed(
           IDR_LOCAL_DISCOVERY_CLOUDPRINT_ICON),
       message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
                                  kPrivetNotificationID),
       product_name, GURL(kPrivetNotificationOriginUrl), kPrivetNotificationID,
-      rich_notification_data, new PrivetNotificationDelegate(profile_));
+      rich_notification_data, CreateNotificationDelegate(profile));
 
   auto* notification_ui_manager = g_browser_process->notification_ui_manager();
-  Profile* profile = Profile::FromBrowserContext(profile_);
   bool updated = notification_ui_manager->Update(notification, profile);
   if (!updated && added &&
       !local_discovery::LocalDiscoveryUIHandler::GetHasVisible()) {
     ReportPrivetUmaEvent(PRIVET_NOTIFICATION_SHOWN);
     notification_ui_manager->Add(notification, profile);
   }
 }
 
 void PrivetNotificationService::PrivetRemoveNotification() {
   ReportPrivetUmaEvent(PRIVET_NOTIFICATION_CANCELED);
@@ skipping 63 matching lines @@
 
   std::unique_ptr<PrivetHTTPAsynchronousFactory> http_factory(
       PrivetHTTPAsynchronousFactory::CreateInstance(
           content::BrowserContext::GetDefaultStoragePartition(profile_)->
               GetURLRequestContext()));
 
   privet_notifications_listener_.reset(
       new PrivetNotificationsListener(std::move(http_factory), this));
 }
 
-PrivetNotificationDelegate::PrivetNotificationDelegate(
-    content::BrowserContext* profile)
-    :  profile_(profile) {
+PrivetNotificationDelegate*
+PrivetNotificationService::CreateNotificationDelegate(Profile* profile) {
+  return new PrivetNotificationDelegate(profile);
 }
 
+PrivetNotificationDelegate::PrivetNotificationDelegate(Profile* profile)
+    : profile_(profile) {}
+
 PrivetNotificationDelegate::~PrivetNotificationDelegate() {
 }
 
 std::string PrivetNotificationDelegate::id() const {
   return kPrivetNotificationID;
 }
 
 void PrivetNotificationDelegate::ButtonClick(int button_index) {
   if (button_index == 0) {
     ReportPrivetUmaEvent(PRIVET_NOTIFICATION_CLICKED);
     OpenTab(GURL(kPrivetNotificationOriginUrl));
-    return;
+  } else {
+    DCHECK_EQ(1, button_index);
+    ReportPrivetUmaEvent(PRIVET_DISABLE_NOTIFICATIONS_CLICKED);
+    DisableNotifications();
   }
-
-  DCHECK_EQ(1, button_index);
-  ReportPrivetUmaEvent(PRIVET_DISABLE_NOTIFICATIONS_CLICKED);
-  DisableNotifications();
+  CloseNotification();

[Lei Zhang, 2016/10/26 00:22:09]

If I move this to the top, like it was in patch set 1, I get a UAF. If I comment
this out, the new unit tests fail when checking the notification count at the
end.

[dewittj, 2016/10/26 16:04:26]

Acknowledged.

 }
 
 void PrivetNotificationDelegate::OpenTab(const GURL& url) {
-  Profile* profile = Profile::FromBrowserContext(profile_);
-  chrome::NavigateParams params(profile, url,
+  chrome::NavigateParams params(profile_, url,
                                 ui::PAGE_TRANSITION_AUTO_TOPLEVEL);
   params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
   chrome::Navigate(&params);
 }
 
 void PrivetNotificationDelegate::DisableNotifications() {
-  Profile* profile = Profile::FromBrowserContext(profile_);
-  profile->GetPrefs()->SetBoolean(prefs::kLocalDiscoveryNotificationsEnabled,
-                                  false);
+  profile_->GetPrefs()->SetBoolean(prefs::kLocalDiscoveryNotificationsEnabled,
+                                   false);
+}
+
+void PrivetNotificationDelegate::CloseNotification() {
+  g_browser_process->notification_ui_manager()->CancelById(
+      id(), NotificationUIManager::GetProfileID(profile_));
 }
 
 }  // namespace cloud_print