Submit a sample of notification images to Safe Browsing

chrome/browser/safe_browsing/ping_manager.h

Index: chrome/browser/safe_browsing/ping_manager.h
diff --git a/chrome/browser/safe_browsing/ping_manager.h b/chrome/browser/safe_browsing/ping_manager.h
index 9f0c1977ce8617790536e7b4ad1326e04f01ae35..7c54e840395e37bba2412fe4a7cc02e5277dc5f2 100644
--- a/chrome/browser/safe_browsing/ping_manager.h
+++ b/chrome/browser/safe_browsing/ping_manager.h
@@ -23,12 +23,15 @@
 #include "net/url_request/url_fetcher_delegate.h"
 #include "url/gurl.h"
 
+class SkBitmap;
+
 namespace net {
 class URLRequestContextGetter;
 }  // namespace net
 
 namespace safe_browsing {
 
+class NotificationImageReporter;
 class PermissionReporter;
 
 class SafeBrowsingPingManager : public net::URLFetcherDelegate {
@@ -55,6 +58,11 @@ class SafeBrowsingPingManager : public net::URLFetcherDelegate {
   // Report permission action to SafeBrowsing servers.
   void ReportPermissionAction(const PermissionReportInfo& report_info);
 
+  // Report notification content image to SafeBrowsing CSD server if necessary.
+  void ReportNotificationImage(Profile* profile,
+                               const GURL& origin,
+                               const SkBitmap& image);
+
  private:
   friend class PermissionReporterBrowserTest;
   friend class SafeBrowsingPingManagerTest;
@@ -102,6 +110,9 @@ class SafeBrowsingPingManager : public net::URLFetcherDelegate {
   // Sends reports of permission actions.
   std::unique_ptr<PermissionReporter> permission_reporter_;
 
+  // Sends reports of notification content images.
+  scoped_refptr<NotificationImageReporter> notification_image_reporter_;

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

Higher level question: we could avoid ref counting if we're OK with dropping
events on the floor when the PingManager is being shut down. (I.e. use a
WeakPtr<>.)

That may be safer too, e.g. is the net::URLRequestContext* passed to the
net::ReportSender assumed to remain valid after the PingManager goes away?

WDYT?

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

I considered this, but it needs to be possible to post to methods on the IO, UI
and blocking pool worker threads. Especially for the latter, there seems no
practical way of satisfying weak pointers' requirement that the weak pointers
are always dereferenced on the same thread.

The lifetime question is relevant though - Nathan, do you know if it's ok that
NotificationImageReporter and its net::ReportSender might outlive the ping
manager due to ref counting?

[Nathan Parker, 2017/01/11 22:34:38]

It feels like a weak_ptr would be safer, since then we'd just drop the report
rather than access a bad URLRequestContext. I think we don't want report_sender_
to outlive PingManager. The PingManager will destruct the
NotificationImageReporter before the URLRequestContextGetter, so that would be
safe.

[johnme, 2017/01/13 02:38:16]

Switched to WeakPtrs. The trick was to only dereference them on the IO thread,
and make the method that run on other threads static (with a weak_this_on_io
parameter which will later get used to post to the non-static IO thread method).

+
   net::NetLogWithSource net_log_;
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingPingManager);