Safe Browsing: Add Omnibox Interaction Incident Reporter

chrome/browser/safe_browsing/incident_reporting/omnibox_watcher.cc

Index: chrome/browser/safe_browsing/incident_reporting/omnibox_watcher.cc
diff --git a/chrome/browser/safe_browsing/incident_reporting/omnibox_watcher.cc b/chrome/browser/safe_browsing/incident_reporting/omnibox_watcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b978abad98917b5a92081b96c79197e43ac80ad7
--- /dev/null
+++ b/chrome/browser/safe_browsing/incident_reporting/omnibox_watcher.cc
@@ -0,0 +1,51 @@
+// Copyright 2014 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/safe_browsing/incident_reporting/omnibox_watcher.h"
+
+#include "base/time/time.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/omnibox/omnibox_log.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "components/omnibox/autocomplete_result.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"
+
+namespace safe_browsing {
+
+OmniboxWatcher::OmniboxWatcher(const AddIncidentCallback& callback):
+    incident_callback_(callback) {
+  registrar_.Add(this, chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
+                 content::NotificationService::AllSources());

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

pass in the Profile* and use content::Source<Profile>(profile) here so that each
instance only monitors the profile its callback is associated with.

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

Good idea.  Done.

+}
+
+OmniboxWatcher::~OmniboxWatcher() {
+}
+
+void OmniboxWatcher::Observe(int type,
+                             const content::NotificationSource& source,
+                             const content::NotificationDetails& details) {
+  DCHECK_EQ(chrome::NOTIFICATION_OMNIBOX_OPENED_URL, type);
+  const OmniboxLog* log = content::Details<OmniboxLog>(details).ptr();
+  const AutocompleteMatch& selected_suggestion =
+      log->result.match_at(log->selected_index);
+  // Users tend not to type very long strings explicitly (especially without
+  // using the paste-and-go option), and certainly not in under a second.
+  // No normal person can type URLs that fast!  Navigating to a URL as a
+  // result of such typing is suspicious.  (Queries are suspicious too--all
+  // such typing is--but we don't monitor queries for now.)

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

consider replacing the parenthetical remark with:
  // TODO(mpearson): Add support for suspicious queries.

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

Done.

+  if (!log->is_paste_and_go && (log->text.length() > 200) &&
+      (log->elapsed_time_since_user_first_modified_omnibox <
+       base::TimeDelta::FromSeconds(1)) &&
+      !AutocompleteMatch::IsSearchType(selected_suggestion.type)) {
+    scoped_ptr<ClientIncidentReport_IncidentData> incident_data(
+        new ClientIncidentReport_IncidentData());
+    const GURL& origin = selected_suggestion.destination_url.GetOrigin();

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

gurl.h says that GetOrigin() may return an empty URL in some circumstances. is
there any possibility of that happening here? if so, should this skip adding the
incident if origin.is_empty()?

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

I don't believe so.  The omnibox suggestions (which technically include the
various types of what-you-typed navigations) all are supposed to have valid
destination URLs.  It's that destination URL that gets broadcast.  I'm pretty
sure we'd have seen other issues in Chrome is sometimes destination URLs are not
valid.

+    incident_data->mutable_omnibox_interaction()->set_origin(
+        origin.possibly_invalid_spec());
+    incident_callback_.Run(incident_data.Pass());
+  }
+}
+
+}  // namespace safe_browsing