Close privet printer notifications when clicked.

chrome/browser/printing/cloud_print/privet_notifications.cc

Index: chrome/browser/printing/cloud_print/privet_notifications.cc
diff --git a/chrome/browser/printing/cloud_print/privet_notifications.cc b/chrome/browser/printing/cloud_print/privet_notifications.cc
index d7eb4e9c691efe352c8e562449b71062b8a4a10e..ba439f34f8f92776e0d9de309b8627af9932e981 100644
--- a/chrome/browser/printing/cloud_print/privet_notifications.cc
+++ b/chrome/browser/printing/cloud_print/privet_notifications.cc
@@ -255,6 +255,7 @@ void PrivetNotificationService::PrivetNotify(int 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(
@@ -262,10 +263,9 @@ void PrivetNotificationService::PrivetNotify(int devices_active,
       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()) {
@@ -349,11 +349,14 @@ void PrivetNotificationService::StartLister() {
       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() {
 }
 
@@ -365,26 +368,29 @@ 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