Submit a sample of notification images to Safe Browsing

chrome/browser/notifications/platform_notification_service_impl.cc

Index: chrome/browser/notifications/platform_notification_service_impl.cc
diff --git a/chrome/browser/notifications/platform_notification_service_impl.cc b/chrome/browser/notifications/platform_notification_service_impl.cc
index 25bcadda2b810d5519e694e293e97e3dee28e966..a84f4e201b97d86d02fd01d1dccb2441cd547a07 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.cc
+++ b/chrome/browser/notifications/platform_notification_service_impl.cc
@@ -21,6 +21,8 @@
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/safe_browsing/ping_manager.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
@@ -36,6 +38,7 @@
 #include "content/public/common/notification_resources.h"
 #include "content/public/common/platform_notification_data.h"
 #include "extensions/features/features.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/message_center/notification.h"
@@ -87,6 +90,18 @@ void CancelNotification(const std::string& notification_id,
                  notification_id));
 }
 
+void ReportNotificationImageOnIOThread(
+    scoped_refptr<safe_browsing::SafeBrowsingService> safe_browsing_service,
+    Profile* profile,
+    const GURL& origin,
+    const SkBitmap& image) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  safe_browsing::SafeBrowsingPingManager* ping_manager =
+      safe_browsing_service->ping_manager();
+  if (ping_manager)
+    ping_manager->ReportNotificationImage(profile, origin, image);

[Peter Beverloo, 2017/01/11 18:23:56]

nit: since initializing and shutting down the ping manager happens on the IO
thread, where as are, you could avoid the null check by checking whether it's
still enabled. That takes away the "why could this be NULL" question from the
reader.
DCHECK_CURRENTLY_ON(BrowserThread::IO);
if (!safe_browsing_service->enabled())
  return;

safe_browsing_service->ping_manager()->ReportNotificationImage(
    profile, origin, image);
(or with a DCHECK if you prefer)

[johnme, 2017/01/11 20:05:07]

Done.

+}
+
 }  // namespace
 
 // static
@@ -428,6 +443,13 @@ Notification PlatformNotificationServiceImpl::CreateNotificationFromData(
     notification.set_type(message_center::NOTIFICATION_TYPE_IMAGE);
     notification.set_image(
         gfx::Image::CreateFrom1xBitmap(notification_resources.image));
+    // n.b. this should only be posted once per notification.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(
+            &ReportNotificationImageOnIOThread,
+            make_scoped_refptr(g_browser_process->safe_browsing_service()),

[Peter Beverloo, 2017/01/11 18:23:56]

I see a bunch of other features doing NULL checks on the
safe_browsing_service(). It appears that you need to call
g_browser_process->SetSafeBrowsingService() when running tests (i.e.
TestingBrowserProcess). Should probably do that too, or ensure that our tests
set one?

[johnme, 2017/01/11 20:05:07]

Done.

+            profile, origin, notification_resources.image));
   }
 
   // Badges are only supported on Android, primarily because it's the only