Experience sampling instrumentation for dangerous downloads warnings

chrome/browser/ui/views/download/download_item_view.h

Index: chrome/browser/ui/views/download/download_item_view.h
diff --git a/chrome/browser/ui/views/download/download_item_view.h b/chrome/browser/ui/views/download/download_item_view.h
index 9920bc47e3d7b8739bf3ab88e3edc125b6a6e1af..14f07aaf428fb70d80cb37f8f9e54842ce13a532 100644
--- a/chrome/browser/ui/views/download/download_item_view.h
+++ b/chrome/browser/ui/views/download/download_item_view.h
@@ -38,6 +38,10 @@
 class DownloadShelfView;
 class DownloadShelfContextMenuView;
 
+namespace extensions {
+class ExperienceSamplingEvent;
+}
+
 namespace gfx {
 class Image;
 class ImageSkia;
@@ -339,6 +343,9 @@ class DownloadItemView : public views::ButtonListener,
   // and reload the icon.
   base::FilePath last_download_item_path_;
 
+  // ExperienceSampling: This maintains event state.

[msw, 2014/08/13 20:11:38]

This comment doesn't really add much value... explain more for the uninformed?

[Chris Thompson, 2014/08/13 21:18:00]

Good point. I've rewritten the comment to explain what it's tracking.

+  scoped_ptr<extensions::ExperienceSamplingEvent> sampling_event_;

[msw, 2014/08/13 20:11:38]

Why isn't all this done at the cross-platform DownloadItemModelData level?
(that might also remove the need for DownloadDangerPromptImpl changes?)

[asanka, 2014/08/13 20:59:30]

Yeah. chrome://downloads also provides a UI surface. It's possible for a user to
open the downloads page from the shelf and then accept/reject the download
there.

What's the lifetime of the sampling_event_ supposed to be?

[Chris Thompson, 2014/08/13 21:18:00]

sampling_event_ should live until the user makes a decision about the warning
(dismisses the download or proceeds all the way through). I could make this more
explicit by null-ing out the scoped_ptr after CreateUserDecisionEvent() is
called.

If the user makes no decision about the UI, the sampling_event_'s lifetime is
tied to the download_item.

Note that we're separately instrumenting the native malicious download warning
prompt, although the results of that prompt (if the user proceeds through) could
also affect the DownloadItemView.

[Chris Thompson, 2014/08/13 21:18:00]

Simpler, cross-platform implementation would be ideal, but there does not seem
to be a good way to track the UI state in that class. We need to be able to
detect when the dangerous/malicious download warning UI is shown, and when the
user responds to it. DownloadItem and DownloadItem::ValidateDangerousDownload
would work, but they're in content/, so we can't instrument them for extensions.

[msw, 2014/08/14 01:48:33]

I wonder if it'd take just a couple new DownloadItemModelData::OnXYZ functions
to support your cross-platform impl, but I'm not adamant that this be done, so
long as you cover all the UI cases that are needed (including
chrome://downloads, Android, iOS, etc.).

+
   DISALLOW_COPY_AND_ASSIGN(DownloadItemView);
 };