Recording download mime types for normal profile

content/browser/download/download_item_impl.cc

Index: content/browser/download/download_item_impl.cc
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index f118a476be598052f23acd1ffe392e303b084e39..8881ff91cbaf84cd9cbbc4b2e48ce73ff3b20e10 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -1158,8 +1158,13 @@ void DownloadItemImpl::Init(bool active,
                             DownloadType download_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  if (active)
+  if (active) {
     RecordDownloadCount(START_COUNT);
+    if (GetWebContents() &&

[asanka, 2017/03/22 21:03:02]

For a new single resource download, Init() gets called before Start(). So at
this point, the DownloadItemImpl doesn't have a request handle. The latter is
what binds the download to a WebContents. I.e. at this point GetWebContents()
will always fail.

Since you just need to check if this is OTR, you can safely call
DownloadItemImpl::GetBrowserContext() during Init(). So your condition would be:

  if (!GetBrowserContext()->IsOffTheRecord())

Even with this, I'd recommend moving this to Start(). Init() is called for
SavePackage downloads for which we are not collecting UMA elsewhere. So your
counts here are going to slightly inconsistent with those taken after Start().

[shaktisahu, 2017/03/22 22:21:36]

Done.

+        !GetWebContents()->GetBrowserContext()->IsOffTheRecord()) {
+      RecordDownloadCount(START_COUNT_NORMAL_PROFILE);
+    }
+  }
 
   std::string file_name;
   if (download_type == SRC_HISTORY_IMPORT) {
@@ -1258,6 +1263,12 @@ void DownloadItemImpl::Start(
     return;
   }
 
+  RecordDownloadMimeType(mime_type_);
+  if (GetWebContents() &&
+      !GetWebContents()->GetBrowserContext()->IsOffTheRecord()) {

[asanka, 2017/03/22 21:03:03]

Call DownloadItemImpl::GetBrowserContext() directly. Here and elsewhere. It's
much more reliable.

Also, are you okay with this recording the MIME type for all download requests
regardless of whether it's a resumption attempt or a new download? You could use
(state_ == RESUMING_INTERNAL) to differentiate between the two cases.

[shaktisahu, 2017/03/22 22:21:36]

Done.
Thanks. I am interested in new downloads only. INITIAL_INTERNAL should filter
that out. Also I moved the START_COUNT metrics from Init() to here. It will be a
bit different from historical data since it doesn't include save package data,
but that should be for the better I guess :)

+    RecordDownloadMimeTypeForNormalProfile(mime_type_);
+  }
+
   // Successful download start.
   DCHECK(download_file_);
   DCHECK(job_);
@@ -1553,6 +1564,10 @@ void DownloadItemImpl::Completed() {
   end_time_ = base::Time::Now();
   TransitionTo(COMPLETE_INTERNAL);
   RecordDownloadCompleted(start_tick_, received_bytes_);
+  if (GetWebContents() &&
+      !GetWebContents()->GetBrowserContext()->IsOffTheRecord()) {
+    RecordDownloadCount(COMPLETED_COUNT_NORMAL_PROFILE);
+  }
 
   if (auto_opened_) {
     // If it was already handled by the delegate, do nothing.