Recording download mime types for normal profile

content/browser/download/download_item_impl.cc

 // Copyright (c) 2012 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.
 
 // File method ordering: Methods in this file are in the same order as
 // in download_item_impl.h, with the following exception: The public
 // interface Start is placed in chronological order with the other
 // (private) routines that together define a DownloadItem's state
 // transitions as the download progresses.  See "Download progression
 // cascade" later in this file.
@@ skipping 1140 matching lines @@
   OnAllDataSaved(total_bytes, std::move(secure_hash));
   MaybeCompleteDownload();
 }
 
 // **** Download progression cascade
 
 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) {
     // target_path_ works for History and Save As versions.
     file_name = target_path_.AsUTF8Unsafe();
   } else {
     // See if it's set programmatically.
     file_name = forced_file_path_.AsUTF8Unsafe();
     // Possibly has a 'download' attribute for the anchor.
     if (file_name.empty())
@@ skipping 78 matching lines @@
 
     // Otherwise, this was a resumption attempt which ended with an
     // interruption. Continue with current target path.
     TransitionTo(TARGET_RESOLVED_INTERNAL);
     InterruptWithPartialState(
         offset, std::move(hash_state), new_create_info.result);
     UpdateObservers();
     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_);
 
   if (state_ == RESUMING_INTERNAL)
     UpdateValidatorsOnResumption(new_create_info);
 
   TransitionTo(TARGET_PENDING_INTERNAL);
 
   job_->Start();
@@ skipping 275 matching lines @@
 
 void DownloadItemImpl::Completed() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   DVLOG(20) << __func__ << "() " << DebugString(false);
 
   DCHECK(all_data_saved_);
   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.
   } else if (GetOpenWhenComplete() ||
              ShouldOpenFileBasedOnExtension() ||
              IsTemporary()) {
     // If the download is temporary, like in drag-and-drop, do not open it but
     // we still need to set it auto-opened so that it can be removed from the
     // download shelf.
     if (!IsTemporary())
@@ skipping 599 matching lines @@
     case RESUME_MODE_USER_CONTINUE:
       return "USER_CONTINUE";
     case RESUME_MODE_USER_RESTART:
       return "USER_RESTART";
   }
   NOTREACHED() << "Unknown resume mode " << mode;
   return "unknown";
 }
 
 }  // namespace content