Add logging to OpenIn feature.

Detect download failure when a corrupted PDF was downloaded and log the
OpenIn download result in all cases.

Bug: 888545
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I19eefd9c1b6da0c9d6b18bfcc0f3953fb33ef391
Reviewed-on: https://chromium-review.googlesource.com/c/1259295Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597328}

ios/chrome/browser/ui/open_in_controller.h

@@ -17,6 +17,17 @@ namespace network {
 class SharedURLLoaderFactory;
 }
 
+// Enum for the IOS.OpenIn.DownloadResult UMA histogram to log the result of
+// the file download initiated when the user tap on "open in" button.
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class OpenInDownloadResult {
+  kSucceeded = 0,
+  kCanceled = 1,
+  kFailed = 2,
+  kMaxValue = kFailed,
+};
+
 @class CRWWebController;
 
 // Class used to handle opening files in other applications.

ios/chrome/browser/ui/open_in_controller.mm

@@ -9,6 +9,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/mac/scoped_cftyperef.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/sequenced_task_runner.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/task/post_task.h"
@@ -62,6 +63,11 @@ const CGFloat kOverlayedViewLabelWidthPercentage = 0.7;
 // Bottom margin for the label displayed on the |overlayedView_|.
 const CGFloat kOverlayedViewLabelBottomMargin = 60;
 
+// Logs the result of the download process after the user taps "open in" button.
+void LogOpenInDownloadResult(const OpenInDownloadResult result) {
+  UMA_HISTOGRAM_ENUMERATION("IOS.OpenIn.DownloadResult", result);
+}
+
 // Returns true if the file located at |url| is a valid PDF file.
 bool HasValidPdfAtUrl(NSURL* _Nullable url) {
   if (!url)
@@ -599,19 +605,23 @@ class OpenInControllerBridge
   if (!filePath.empty())
     fileURL = [NSURL fileURLWithPath:base::SysUTF8ToNSString(filePath.value())];
   if (!downloadCanceled_ && HasValidPdfAtUrl(fileURL)) {
+    LogOpenInDownloadResult(OpenInDownloadResult::kSucceeded);
     [self presentOpenInMenuForFileAtURL:fileURL];
     return;
   }
   sequencedTaskRunner_->PostTask(FROM_HERE, base::BindOnce(^{
                                    [self removeDocumentAtPath:fileURL.path];
                                  }));
+  OpenInDownloadResult download_result = OpenInDownloadResult::kCanceled;
   if (!downloadCanceled_) {
+    download_result = OpenInDownloadResult::kFailed;
     if (IsIPadIdiom())
       [self hideOpenInToolbar];
     [self removeOverlayedView];
     [self showErrorWithMessage:l10n_util::GetNSStringWithFixup(
                                    IDS_IOS_OPEN_IN_FILE_DOWNLOAD_FAILED)];
   }
+  LogOpenInDownloadResult(download_result);
 }
 
 #pragma mark -

ios/chrome/browser/ui/open_in_controller_unittest.mm

@@ -8,6 +8,7 @@
 
 #include "base/files/file_util.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_task_environment.h"
 #import "ios/chrome/browser/tabs/tab.h"
 #import "ios/chrome/browser/ui/open_in_controller.h"
@@ -28,6 +29,8 @@
 
 namespace {
 
+const char kOpenInDownloadResultHistogram[] = "IOS.OpenIn.DownloadResult";
+
 class OpenInControllerTest : public PlatformTest {
  public:
   OpenInControllerTest()
@@ -80,9 +83,11 @@ class OpenInControllerTest : public PlatformTest {
   net::TestURLFetcherFactory factory_;
   OpenInController* open_in_controller_;
   UIView* parent_view_;
+  base::HistogramTester histogram_tester_;
 };
 
 TEST_F(OpenInControllerTest, TestDisplayOpenInMenu) {
+  histogram_tester_.ExpectTotalCount(kOpenInDownloadResultHistogram, 0);
   id document_controller =
       [OCMockObject niceMockForClass:[UIDocumentInteractionController class]];
   [open_in_controller_ setDocumentInteractionController:document_controller];
@@ -99,11 +104,16 @@ TEST_F(OpenInControllerTest, TestDisplayOpenInMenu) {
   test_url_loader_factory_.SimulateResponseForPendingRequest(
       pending_request->request.url.spec(), pdf_str);
   scoped_task_environment_.RunUntilIdle();
-
+  histogram_tester_.ExpectBucketCount(kOpenInDownloadResultHistogram,
+                                      static_cast<base::HistogramBase::Sample>(
+                                          OpenInDownloadResult::kSucceeded),
+                                      1);
+  histogram_tester_.ExpectTotalCount(kOpenInDownloadResultHistogram, 1);
   EXPECT_OCMOCK_VERIFY(document_controller);
 }
 
 TEST_F(OpenInControllerTest, TestCorruptedPDFDownload) {
+  histogram_tester_.ExpectTotalCount(kOpenInDownloadResultHistogram, 0);
   id document_controller =
       [OCMockObject niceMockForClass:[UIDocumentInteractionController class]];
   [open_in_controller_ setDocumentInteractionController:document_controller];
@@ -119,7 +129,11 @@ TEST_F(OpenInControllerTest, TestCorruptedPDFDownload) {
   test_url_loader_factory_.SimulateResponseForPendingRequest(
       pending_request->request.url.spec(), pdf_str.substr(pdf_str.size() / 2));
   scoped_task_environment_.RunUntilIdle();
-
+  histogram_tester_.ExpectBucketCount(
+      kOpenInDownloadResultHistogram,
+      static_cast<base::HistogramBase::Sample>(OpenInDownloadResult::kFailed),
+      1);
+  histogram_tester_.ExpectTotalCount(kOpenInDownloadResultHistogram, 1);
   EXPECT_OCMOCK_VERIFY(document_controller);
 }
 

tools/metrics/histograms/enums.xml

@@ -26495,6 +26495,16 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="2" label="Remote suggestions collapsed"/>
 </enum>
 
+<enum name="IOSOpenInDownloadResult">
+  <int value="0" label="Download Succeeded"/>
+  <int value="1" label="Download Canceled">
+    User canceled the download manually.
+  </int>
+  <int value="2" label="Download Failed">
+    Download failed either by getting a corrupted file or no file at all.
+  </int>
+</enum>
+
 <enum name="IOSPageLoadCountNavigationType">
   <int value="0" label="Chrome URL Navigation"/>
   <int value="1" label="Same Document Web Navigation"/>

tools/metrics/histograms/histograms.xml

@@ -39347,6 +39347,14 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="IOS.OpenIn.DownloadResult" enum="IOSOpenInDownloadResult">
+  <owner>mrefaat@chromium.org</owner>
+  <summary>
+    The result of the download operation done when the user taps &quot;open
+    in&quot; button to open a file by a different application.
+  </summary>
+</histogram>
+
 <histogram name="IOS.PageLoadCount.Counts"
     enum="IOSPageLoadCountNavigationType">
   <owner>danyao@chromium.org</owner>