Safe Browsing: Add Omnibox Interaction Incident Reporter

chrome/browser/safe_browsing/safe_browsing_service.h

Index: chrome/browser/safe_browsing/safe_browsing_service.h
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index 6230e7ac60746245ecf5dc1fc8cda5456c85d7f3..6e4537f268193b5c3df2c6081a2a6fdf13b41aa5 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -58,6 +58,7 @@ class DownloadProtectionService;
 
 #if defined(FULL_SAFE_BROWSING)
 class IncidentReportingService;
+class OmniboxWatcher;
 class OffDomainInclusionDetector;
 #endif
 }
@@ -206,6 +207,12 @@ class SafeBrowsingService
   // Stop following the safe browsing preference on |pref_service|.
   void RemovePrefService(PrefService* pref_service);
 
+  // Creates a OmniboxWatcher for the newly-instantiated |profile|.
+  void AddOmniboxWatcher(Profile* profile);
+
+  // Deletes the OmniboxWatcher associated with |profile|.
+  void RemoveOmniboxWatcher(Profile* profile);
+
   // Checks if any profile is currently using the safe browsing service, and
   // starts or stops the service accordingly.
   void RefreshState();
@@ -239,6 +246,9 @@ class SafeBrowsingService
   // Accessed on UI thread.
   std::map<PrefService*, PrefChangeRegistrar*> prefs_map_;
 
+  // Tracks existing OmniboxWatchers. This is used to keep one per profile.
+  std::map<Profile*, safe_browsing::OmniboxWatcher*> omnibox_watcher_map_;

[grt (UTC plus 2), 2014/12/05 18:40:56]

how about putting a scoped_ptr<OmniboxWatcher> member on
IncidentReportingService::ProfileContext instead of having a new collection
here? i have some reservations about mixing incident handlers in there, but it
seems like such a no-brainer to have IncidentReportingService::OnProfileAdded
create the watcher and let it die when the ProfileContext is deleted in
IncidentReportingService::OnProfileDestroyed.

[Mark P, 2014/12/09 19:23:30]

Done.

It is okay that one listens for NOTIFICATION_PROFILE_ADDED and one listens for
NOTIFICATION_PROFILE_CREATED?  I don't think these are equivalent.

[Mark P, 2014/12/09 19:24:48]

Done.

I love how I can offhandedly mention any issue that I'm having when I leave work
on day, and, before I spend much time thinking about it, I have the solution in
my inbox in the morning. :-)

It is okay that one listens for NOTIFICATION_PROFILE_ADDED and one listens for
NOTIFICATION_PROFILE_CREATED?  I don't think these are equivalent.

[grt (UTC plus 2), 2014/12/09 20:15:52]

I think it's okay. PROFILE_ADDED happens when the Profile is added to the
ProfileManager. It likely happens a little later than _CREATED, but still before
the omnibox can be interacted with.

noms: Are there any cases where NOTIFICATION_PROFILE_CREATED can be sent for a
profile, but not NOTIFICATION_PROFILE_ADDED? More to the point, is it possible
for a user to interact with the omnibox before PROFILE_ADDED is called?

[noms (inactive), 2014/12/09 20:35:52]

Hmmm, so I think that this can happen for sure in tests (you manually create a
Profile, but you don't add it to the ProfileManager). In a regular Chrome run, I
think it would only happen if something's gone horribly wrong: Chrome crashes
after the Profile was created, the ProfileManager gets itself corrupted...all
situations when all bets are off anyway.

I didn't check who listens to PROFILE_CREATED, but keep in mind at that point
all PKSes and stuff like that aren't initialized for the Profile yet -- those
get done after PROFILE_CREATED and before PROFILE_ADDED. 

[grt (UTC plus 2), 2014/12/10 15:13:16]

Thanks for the info. I think this is okay, then.

+
   // Used to track creation and destruction of profiles on the UI thread.
   content::NotificationRegistrar prefs_registrar_;