Bug 1592934 - Add a telemetry probe to track temporary storage initialization success rate; r=janv,janerik a=pascalc
authorTom Tung <ttung@mozilla.com>
Thu, 21 Nov 2019 07:59:34 +0000
changeset 563469 63d69f4dfafebd03825e41bbdb6b08e5b36386e4
parent 563468 c12729e7c41f81f24d9581b7b1bf8c20cc485526
child 563470 0376c52e0a808922d35a0b3646e828622563c56e
push id2195
push userffxbld-merge
push dateMon, 25 Nov 2019 12:02:33 +0000
treeherdermozilla-release@19adee6f7bb3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjanv, janerik, pascalc
bugs1592934
milestone71.0
Bug 1592934 - Add a telemetry probe to track temporary storage initialization success rate; r=janv,janerik a=pascalc Differential Revision: https://phabricator.services.mozilla.com/D51267
dom/quota/ActorsParent.cpp
dom/quota/QuotaManager.h
dom/quota/test/unit/test_qm_first_initialization_attempt.js
dom/quota/test/unit/xpcshell.ini
toolkit/components/telemetry/Histograms.json
--- a/dom/quota/ActorsParent.cpp
+++ b/dom/quota/ActorsParent.cpp
@@ -189,16 +189,19 @@ static_assert(static_cast<uint32_t>(Stor
                   static_cast<uint32_t>(PERSISTENCE_TYPE_DEFAULT),
               "Enum values should match.");
 
 const char kChromeOrigin[] = "chrome";
 const char kAboutHomeOriginPrefix[] = "moz-safe-about:home";
 const char kIndexedDBOriginPrefix[] = "indexeddb://";
 const char kResourceOriginPrefix[] = "resource://";
 
+constexpr auto kTempStorageTelemetryKey =
+    NS_LITERAL_CSTRING("TemporaryStorage");
+
 #define INDEXEDDB_DIRECTORY_NAME "indexedDB"
 #define STORAGE_DIRECTORY_NAME "storage"
 #define PERSISTENT_DIRECTORY_NAME "persistent"
 #define PERMANENT_DIRECTORY_NAME "permanent"
 #define TEMPORARY_DIRECTORY_NAME "temporary"
 #define DEFAULT_DIRECTORY_NAME "default"
 
 #define STORAGE_FILE_NAME "storage.sqlite"
@@ -3424,16 +3427,18 @@ bool QuotaObject::LockedMaybeUpdateSize(
 /*******************************************************************************
  * Quota manager
  ******************************************************************************/
 
 QuotaManager::QuotaManager()
     : mQuotaMutex("QuotaManager.mQuotaMutex"),
       mTemporaryStorageLimit(0),
       mTemporaryStorageUsage(0),
+      mNextDirectoryLockId(0),
+      mTemporaryStorageInitializationAttempted(false),
       mTemporaryStorageInitialized(false),
       mCacheUsable(false) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(!gInstance);
 }
 
 QuotaManager::~QuotaManager() {
   AssertIsOnOwningThread();
@@ -6778,19 +6783,32 @@ nsresult QuotaManager::EnsureOriginIsIni
   return NS_OK;
 }
 
 nsresult QuotaManager::EnsureTemporaryStorageIsInitialized() {
   AssertIsOnIOThread();
   MOZ_ASSERT(mStorageConnection);
 
   if (mTemporaryStorageInitialized) {
+    MOZ_ASSERT(mTemporaryStorageInitializationAttempted);
     return NS_OK;
   }
 
+  auto autoReportTelemetry = MakeScopeExit([&]() {
+    Telemetry::Accumulate(Telemetry::QM_FIRST_INITIALIZATION_ATTEMPT,
+                          kTempStorageTelemetryKey,
+                          static_cast<uint32_t>(mTemporaryStorageInitialized));
+  });
+
+  if (mTemporaryStorageInitializationAttempted) {
+    autoReportTelemetry.release();
+  }
+
+  mTemporaryStorageInitializationAttempted = true;
+
   nsresult rv;
 
   nsCOMPtr<nsIFile> storageDir =
       do_CreateInstance(NS_LOCAL_FILE_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
@@ -6847,16 +6865,18 @@ void QuotaManager::ShutdownStorage() {
         UnloadQuota();
       } else {
         RemoveQuota();
       }
 
       mTemporaryStorageInitialized = false;
     }
 
+    mTemporaryStorageInitializationAttempted = false;
+
     ReleaseIOThreadObjects();
 
     mStorageConnection = nullptr;
   }
 }
 
 nsresult QuotaManager::EnsureOriginDirectory(nsIFile* aDirectory,
                                              bool* aCreated) {
--- a/dom/quota/QuotaManager.h
+++ b/dom/quota/QuotaManager.h
@@ -589,15 +589,17 @@ class QuotaManager final : public Backgr
   nsString mIndexedDBPath;
   nsString mStoragePath;
   nsString mPermanentStoragePath;
   nsString mTemporaryStoragePath;
   nsString mDefaultStoragePath;
 
   uint64_t mTemporaryStorageLimit;
   uint64_t mTemporaryStorageUsage;
+  int64_t mNextDirectoryLockId;
+  bool mTemporaryStorageInitializationAttempted;
   bool mTemporaryStorageInitialized;
   bool mCacheUsable;
 };
 
 END_QUOTA_NAMESPACE
 
 #endif /* mozilla_dom_quota_quotamanager_h__ */
new file mode 100644
--- /dev/null
+++ b/dom/quota/test/unit/test_qm_first_initialization_attempt.js
@@ -0,0 +1,123 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+const { TelemetryTestUtils } = ChromeUtils.import(
+  "resource://testing-common/TelemetryTestUtils.jsm"
+);
+
+const telemetry = "QM_FIRST_INITIALIZATION_ATTEMPT";
+
+const testcases = [
+  {
+    key: "TemporaryStorage",
+    testingInitFunction() {
+      return initTemporaryStorage();
+    },
+    get metadataDir() {
+      return getRelativeFile(
+        "storage/default/https+++example.com/.metadata-v2"
+      );
+    },
+    async settingForForcingInitFailure() {
+      // We need to initialize storage before creating the metadata directory.
+      // If we don't do that, the storage/ directory created for the metadata
+      // directory would trigger storage upgrades (from version 0 to current
+      // version) which would fail due to the metadata directory entry being
+      // a directory (not a file).
+      let request = init();
+      await requestFinished(request);
+
+      this.metadataDir.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
+    },
+    removeSetting() {
+      this.metadataDir.remove(false);
+    },
+    expectedResult: {
+      initFailure: [1, 0],
+      initFailureThenSuccess: [1, 1, 0],
+    },
+  },
+];
+
+function verifyResult(histogram, key, expectedResult) {
+  const snapshot = histogram.snapshot();
+
+  ok(key in snapshot, `The histogram ${histogram.name()} must contain ${key}`);
+
+  is(
+    Object.entries(snapshot[key].values).length,
+    expectedResult.length,
+    `Reported telemetry should have the same size as expected result (${
+      expectedResult.length
+    })`
+  );
+
+  for (let i = 0; i < expectedResult.length; ++i) {
+    is(
+      snapshot[key].values[i],
+      expectedResult[i],
+      `Expected counts should match for ${histogram.name()} at index ${i}`
+    );
+  }
+}
+
+async function testSteps() {
+  let request;
+  for (let testcase of testcases) {
+    const key = testcase.key;
+
+    info("Verifying the telemetry probe " + telemetry + " for the key " + key);
+
+    const histogram = TelemetryTestUtils.getAndClearKeyedHistogram(telemetry);
+
+    for (let expectedInitResult of [false, true]) {
+      info(
+        "Verifying the reported result on the Telemetry is expected when " +
+          "the init " +
+          (expectedInitResult ? "succeeds" : "fails")
+      );
+
+      if (!expectedInitResult) {
+        await testcase.settingForForcingInitFailure();
+      }
+
+      const msg =
+        "Should " + (expectedInitResult ? "not " : "") + "have thrown";
+
+      // The steps below verify we should get the result of the first attempt
+      // for the initialization.
+      for (let i = 0; i < 2; ++i) {
+        request = testcase.testingInitFunction();
+        try {
+          await requestFinished(request);
+          ok(expectedInitResult, msg);
+        } catch (ex) {
+          ok(!expectedInitResult, msg);
+        }
+      }
+
+      if (!expectedInitResult) {
+        // Remove the setting (e.g. files) created by the
+        // settingForForcingInitFilaure(). We need to do that for the next
+        // iteration in which the initialization is supposed to succeed.
+        testcase.removeSetting();
+      }
+
+      const expectedResultForThisRun = expectedInitResult
+        ? testcase.expectedResult.initFailureThenSuccess
+        : testcase.expectedResult.initFailure;
+      verifyResult(histogram, key, expectedResultForThisRun);
+
+      // The first initialization attempt has been reported in telemetry and
+      // any new attemps wouldn't be reported if we don't reset the storage
+      // here.
+      request = reset();
+      await requestFinished(request);
+    }
+
+    request = clear();
+    await requestFinished(request);
+  }
+}
--- a/dom/quota/test/unit/xpcshell.ini
+++ b/dom/quota/test/unit/xpcshell.ini
@@ -46,16 +46,17 @@ support-files =
 [test_obsoleteOriginAttributesUpgrade.js]
 [test_obsoleteOrigins.js]
 [test_originAttributesUpgrade.js]
 [test_originWithCaret.js]
 [test_persist.js]
 [test_persist_eviction.js]
 [test_persist_globalLimit.js]
 [test_persist_groupLimit.js]
+[test_qm_first_initialization_attempt.js]
 [test_removeLocalStorage.js]
 [test_simpledb.js]
 [test_specialOrigins.js]
 [test_storagePersistentUpgrade.js]
 [test_storagePressure.js]
 [test_tempMetadataCleanup.js]
 [test_unknownFiles.js]
 [test_validOrigins.js]
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -15266,16 +15266,27 @@
     "bug_numbers": [1580238],
     "kind": "exponential",
     "high": 120000,
     "n_buckets": 60,
     "releaseChannelCollection": "opt-out",
     "alert_emails": ["ttung@mozilla.com"],
     "description": "Time (ms) for the QuotaManager to initialize repositories."
   },
+  "QM_FIRST_INITIALIZATION_ATTEMPT": {
+    "record_in_processes": ["main"],
+    "products": ["firefox", "fennec", "geckoview"],
+    "expires_in_version": "never",
+    "bug_numbers": [1592934],
+    "kind": "boolean",
+    "keyed": true,
+    "releaseChannelCollection": "opt-out",
+    "alert_emails": ["ttung@mozilla.com", "storage-telemetry@mozilla.com"],
+    "description": "True if the first initialization attempt succeeded, keyed by the initialization type."
+  },
   "GV_PAGE_LOAD_PROGRESS_MS": {
     "record_in_processes": ["main", "content"],
     "products": ["firefox", "fennec", "geckoview"],
     "alert_emails": [
       "geckoview-team@mozilla.com",
       "esawin@mozilla.com"
     ],
     "expires_in_version": "never",