indexeddb: emit metrics for blob files not being found

It is unclear what the rate of blob files missing are as a part of this
bug, and so add some metrics to try to get a handle on the scope of the
problem.

FilesystemProxyFileStreamReader is currently only used for
IndexedDBDataItemReader, but this creates a separate entry point and
metrics boolean in case this class gets reused for something other than
IndexedDB.

Bug: 1131151
Change-Id: I4d0610413534fb40edb95e321b6d58e92fdf23ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446889Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Auto-Submit: enne <enne@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814286}

storage/browser/file_system/file_stream_reader.h

@@ -62,6 +62,15 @@ class FileStreamReader {
       int64_t initial_offset,
       const base::Time& expected_modification_time);
 
+  // The same as CreateForFilesystemProxy, but will emit diagnostic metrics.
+  COMPONENT_EXPORT(STORAGE_BROWSER)
+  static std::unique_ptr<FileStreamReader> CreateForIndexedDBDataItemReader(
+      scoped_refptr<base::TaskRunner> task_runner,
+      const base::FilePath& file_path,
+      std::unique_ptr<storage::FilesystemProxy> filesystem_proxy,
+      int64_t initial_offset,
+      const base::Time& expected_modification_time);
+
   // Creates a new FileReader for a memory file |file_path|.
   // |initial_offset| specifies the offset in the file where the first read
   // should start.  If the given offset is out of the file range any

storage/browser/file_system/filesystem_proxy_file_stream_reader.cc

@@ -13,6 +13,7 @@
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/task_runner.h"
 #include "base/task_runner_util.h"
 #include "net/base/file_stream.h"
@@ -62,9 +63,24 @@ std::unique_ptr<FileStreamReader> FileStreamReader::CreateForFilesystemProxy(
     int64_t initial_offset,
     const base::Time& expected_modification_time) {
   DCHECK(filesystem_proxy);
+  constexpr bool emit_metrics = false;
   return base::WrapUnique(new FilesystemProxyFileStreamReader(
       std::move(task_runner), file_path, std::move(filesystem_proxy),
-      initial_offset, expected_modification_time));
+      initial_offset, expected_modification_time, emit_metrics));
+}
+
+std::unique_ptr<FileStreamReader>
+FileStreamReader::CreateForIndexedDBDataItemReader(
+    scoped_refptr<base::TaskRunner> task_runner,
+    const base::FilePath& file_path,
+    std::unique_ptr<storage::FilesystemProxy> filesystem_proxy,
+    int64_t initial_offset,
+    const base::Time& expected_modification_time) {
+  DCHECK(filesystem_proxy);
+  constexpr bool emit_metrics = true;
+  return base::WrapUnique(new FilesystemProxyFileStreamReader(
+      std::move(task_runner), file_path, std::move(filesystem_proxy),
+      initial_offset, expected_modification_time, emit_metrics));
 }
 
 FilesystemProxyFileStreamReader::~FilesystemProxyFileStreamReader() = default;
@@ -101,13 +117,15 @@ FilesystemProxyFileStreamReader::FilesystemProxyFileStreamReader(
     const base::FilePath& file_path,
     std::unique_ptr<storage::FilesystemProxy> filesystem_proxy,
     int64_t initial_offset,
-    const base::Time& expected_modification_time)
+    const base::Time& expected_modification_time,
+    bool emit_metrics)
     : task_runner_(std::move(task_runner)),
       shared_filesystem_proxy_(base::MakeRefCounted<SharedFilesystemProxy>(
           std::move(filesystem_proxy))),
       file_path_(file_path),
       initial_offset_(initial_offset),
-      expected_modification_time_(expected_modification_time) {}
+      expected_modification_time_(expected_modification_time),
+      emit_metrics_(emit_metrics) {}
 
 void FilesystemProxyFileStreamReader::Open(
     net::CompletionOnceCallback callback) {
@@ -195,6 +213,14 @@ void FilesystemProxyFileStreamReader::DidOpenForRead(
 void FilesystemProxyFileStreamReader::DidGetFileInfoForGetLength(
     net::Int64CompletionOnceCallback callback,
     FileErrorOr<base::File::Info> result) {
+  // TODO(enne): track rate of missing blobs for http://crbug.com/1131151
+  if (emit_metrics_) {
+    bool file_was_found = !result.is_error() ||
+                          result.error() != base::File::FILE_ERROR_NOT_FOUND;
+    UMA_HISTOGRAM_BOOLEAN("WebCore.IndexedDB.FoundBlobFileForValue",
+                          file_was_found);
+  }
+
   if (result.is_error()) {
     std::move(callback).Run(net::FileErrorToNetError(result.error()));
     return;

storage/browser/file_system/filesystem_proxy_file_stream_reader.h

@@ -53,7 +53,8 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) FilesystemProxyFileStreamReader
       const base::FilePath& file_path,
       std::unique_ptr<storage::FilesystemProxy> filesystem_proxy,
       int64_t initial_offset,
-      const base::Time& expected_modification_time);
+      const base::Time& expected_modification_time,
+      bool emit_metrics);
   void Open(net::CompletionOnceCallback callback);
 
   // Callbacks that are chained from Open for Read.
@@ -83,6 +84,7 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) FilesystemProxyFileStreamReader
   const int64_t initial_offset_;
   const base::Time expected_modification_time_;
   bool has_pending_open_ = false;
+  bool emit_metrics_ = false;
   base::WeakPtrFactory<FilesystemProxyFileStreamReader> weak_factory_{this};
 };
 

tools/metrics/histograms/histograms_xml/web_core/histograms.xml

@@ -304,6 +304,19 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="WebCore.IndexedDB.FoundBlobFileForValue" enum="Boolean"
+    expires_after="M95">
+  <owner>enne@chromium.org</owner>
+  <owner>mek@chromium.org</owner>
+  <summary>
+    Recorded when a blob is attempted to be read from an IndexedDB value. This
+    is triggered for both implicit blob-wrapped large values and explicit blob
+    values. Its value is false iff there was a file not found value when reading
+    the file. This metric is to help track down the rate of missing files for
+    http://crbug.com/1131151.
+  </summary>
+</histogram>
+
 <histogram name="WebCore.IndexedDB.LevelDB.CloseTime" units="ms"
     expires_after="M95">
   <owner>cmumford@chromium.org</owner>