[Paint Preview] DirectoryKey for file management

Refactor FileManager to accept a DirectoryKey rather than a GURL. This
makes it more ammenable to possibly supporting other keys for creating
directories such as tab id, UnguessableTokens, etc.

Bug: 1055505
Change-Id: Id93cc1bcbfdfdd69baf0f57764636dcbcd62c3f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2070737Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Mehran Mahmoudi <mahmoudi@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745050}

components/paint_preview/browser/android/paint_preview_utils.cc

@@ -82,8 +82,9 @@ void CompressAndMeasureSize(const base::FilePath& root_dir,
                             FinishedCallback finished,
                             bool keep_zip) {
   FileManager manager(root_dir);
+  auto key = manager.CreateKey(url);
   base::FilePath path;
-  bool success = manager.CreateOrGetDirectoryFor(url, &path);
+  bool success = manager.CreateOrGetDirectory(key, &path);
   if (!success) {
     VLOG(1) << kPaintPreviewTestTag << "Failure: could not find url dir.";
     CleanupOnFailure(root_dir, std::move(finished));
@@ -93,8 +94,8 @@ void CompressAndMeasureSize(const base::FilePath& root_dir,
                   base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
   std::string str_proto = proto->SerializeAsString();
   file.WriteAtCurrentPos(str_proto.data(), str_proto.size());
-  manager.CompressDirectoryFor(url);
-  metrics.compressed_size_bytes = manager.GetSizeOfArtifactsFor(url);
+  manager.CompressDirectory(key);
+  metrics.compressed_size_bytes = manager.GetSizeOfArtifacts(key);
 
   CleanupAndLogResult(path.AddExtensionASCII("zip"), metrics,
                       std::move(finished), keep_zip);
@@ -133,7 +134,7 @@ base::Optional<base::FilePath> CreateDirectoryForURL(
     const GURL& url) {
   base::FilePath url_path;
   FileManager manager(root_path);
-  if (!manager.CreateOrGetDirectoryFor(url, &url_path)) {
+  if (!manager.CreateOrGetDirectory(manager.CreateKey(url), &url_path)) {
     VLOG(1) << kPaintPreviewTestTag << "Failure: could not create output dir.";
     return base::nullopt;
   }

components/paint_preview/browser/file_manager.cc

@@ -7,7 +7,6 @@
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/hash/hash.h"
-#include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "third_party/zlib/google/zip.h"
 
@@ -17,24 +16,29 @@ namespace {
 
 constexpr char kZipExt[] = ".zip";
 
-std::string HashToHex(const GURL& url) {
-  uint32_t hash = base::PersistentHash(url.spec());
-  return base::HexEncode(&hash, sizeof(uint32_t));
-}
-
 }  // namespace
 
 FileManager::FileManager(const base::FilePath& root_directory)
     : root_directory_(root_directory) {}
+
 FileManager::~FileManager() = default;
 
-size_t FileManager::GetSizeOfArtifactsFor(const GURL& url) {
+DirectoryKey FileManager::CreateKey(const GURL& url) const {
+  uint32_t hash = base::PersistentHash(url.spec());
+  return DirectoryKey{base::HexEncode(&hash, sizeof(uint32_t))};
+}
+
+DirectoryKey FileManager::CreateKey(uint64_t tab_id) const {
+  return DirectoryKey{base::NumberToString(tab_id)};
+}
+
+size_t FileManager::GetSizeOfArtifacts(const DirectoryKey& key) {
   base::FilePath path;
-  StorageType storage_type = GetPathForUrl(url, &path);
+  StorageType storage_type = GetPathForKey(key, &path);
   switch (storage_type) {
     case kDirectory: {
       return base::ComputeDirectorySize(
-          root_directory_.AppendASCII(HashToHex(url)));
+          root_directory_.AppendASCII(key.ascii_dirname));
     }
     case kZip: {
       int64_t file_size = 0;
@@ -48,9 +52,10 @@ size_t FileManager::GetSizeOfArtifactsFor(const GURL& url) {
   }
 }
 
-bool FileManager::GetCreatedTime(const GURL& url, base::Time* created_time) {
+bool FileManager::GetCreatedTime(const DirectoryKey& key,
+                                 base::Time* created_time) {
   base::FilePath path;
-  StorageType storage_type = GetPathForUrl(url, &path);
+  StorageType storage_type = GetPathForKey(key, &path);
   if (storage_type == FileManager::StorageType::kNone)
     return false;
   base::File::Info info;
@@ -60,10 +65,10 @@ bool FileManager::GetCreatedTime(const GURL& url, base::Time* created_time) {
   return true;
 }
 
-bool FileManager::GetLastModifiedTime(const GURL& url,
+bool FileManager::GetLastModifiedTime(const DirectoryKey& key,
                                       base::Time* last_modified_time) {
   base::FilePath path;
-  StorageType storage_type = GetPathForUrl(url, &path);
+  StorageType storage_type = GetPathForKey(key, &path);
   if (storage_type == FileManager::StorageType::kNone)
     return false;
   base::File::Info info;
@@ -73,13 +78,18 @@ bool FileManager::GetLastModifiedTime(const GURL& url,
   return true;
 }
 
-bool FileManager::CreateOrGetDirectoryFor(const GURL& url,
-                                          base::FilePath* directory) {
+bool FileManager::DirectoryExists(const DirectoryKey& key) {
   base::FilePath path;
-  StorageType storage_type = GetPathForUrl(url, &path);
+  return GetPathForKey(key, &path) != StorageType::kNone;
+}
+
+bool FileManager::CreateOrGetDirectory(const DirectoryKey& key,
+                                       base::FilePath* directory) {
+  base::FilePath path;
+  StorageType storage_type = GetPathForKey(key, &path);
   switch (storage_type) {
     case kNone: {
-      base::FilePath new_path = root_directory_.AppendASCII(HashToHex(url));
+      base::FilePath new_path = root_directory_.AppendASCII(key.ascii_dirname);
       base::File::Error error = base::File::FILE_OK;
       if (base::CreateDirectoryAndGetError(new_path, &error)) {
         *directory = new_path;
@@ -94,7 +104,7 @@ bool FileManager::CreateOrGetDirectoryFor(const GURL& url,
       return true;
     }
     case kZip: {
-      base::FilePath dst_path = root_directory_.AppendASCII(HashToHex(url));
+      base::FilePath dst_path = root_directory_.AppendASCII(key.ascii_dirname);
       base::File::Error error = base::File::FILE_OK;
       if (!base::CreateDirectoryAndGetError(dst_path, &error)) {
         DVLOG(1) << "ERROR: failed to create directory: " << path
@@ -114,9 +124,9 @@ bool FileManager::CreateOrGetDirectoryFor(const GURL& url,
   }
 }
 
-bool FileManager::CompressDirectoryFor(const GURL& url) {
+bool FileManager::CompressDirectory(const DirectoryKey& key) {
   base::FilePath path;
-  StorageType storage_type = GetPathForUrl(url, &path);
+  StorageType storage_type = GetPathForKey(key, &path);
   switch (storage_type) {
     case kDirectory: {
       // If there are no files in the directory, zip will succeed, but unzip
@@ -137,23 +147,27 @@ bool FileManager::CompressDirectoryFor(const GURL& url) {
   }
 }
 
-void FileManager::DeleteArtifactsFor(const std::vector<GURL>& urls) {
-  for (const auto& url : urls) {
-    base::FilePath path;
-    StorageType storage_type = GetPathForUrl(url, &path);
-    if (storage_type == FileManager::StorageType::kNone)
-      continue;
-    base::DeleteFileRecursively(path);
-  }
+void FileManager::DeleteArtifacts(const DirectoryKey& key) {
+  base::FilePath path;
+  StorageType storage_type = GetPathForKey(key, &path);
+  if (storage_type == FileManager::StorageType::kNone)
+    return;
+  base::DeleteFileRecursively(path);
+}
+
+void FileManager::DeleteArtifacts(const std::vector<DirectoryKey>& keys) {
+  for (const auto& key : keys)
+    DeleteArtifacts(key);
 }
 
 void FileManager::DeleteAll() {
   base::DeleteFileRecursively(root_directory_);
 }
 
-FileManager::StorageType FileManager::GetPathForUrl(const GURL& url,
+FileManager::StorageType FileManager::GetPathForKey(const DirectoryKey& key,
                                                     base::FilePath* path) {
-  base::FilePath directory_path = root_directory_.AppendASCII(HashToHex(url));
+  base::FilePath directory_path =
+      root_directory_.AppendASCII(key.ascii_dirname);
   if (base::PathExists(directory_path)) {
     *path = directory_path;
     return kDirectory;

components/paint_preview/browser/file_manager.h

@@ -5,14 +5,20 @@
 #ifndef COMPONENTS_PAINT_PREVIEW_BROWSER_FILE_MANAGER_H_
 #define COMPONENTS_PAINT_PREVIEW_BROWSER_FILE_MANAGER_H_
 
+#include <string>
+
 #include "base/files/file_path.h"
 #include "base/time/time.h"
 #include "url/gurl.h"
 
 namespace paint_preview {
 
-// Manages paint preview files associated with a root directory (typically a
-// user profile).
+struct DirectoryKey {
+  const std::string ascii_dirname;
+};
+
+// Manages paint preview files associated with a root directory typically the
+// root directory is <profile_dir>/paint_previews/<feature>.
 class FileManager {
  public:
   // Create a file manager for |root_directory|. Top level items in
@@ -21,26 +27,40 @@ class FileManager {
   explicit FileManager(const base::FilePath& root_directory);
   ~FileManager();
 
+  FileManager(const FileManager&) = delete;
+  FileManager& operator=(const FileManager&) = delete;
+
+  // Creates a DirectoryKey from keying material.
+  // TODO(crbug/1056226): implement collision resolution. At present collisions
+  // result in overwriting data.
+  DirectoryKey CreateKey(const GURL& url) const;
+  DirectoryKey CreateKey(uint64_t tab_id) const;
+
   // Get statistics about the time of creation and size of artifacts.
-  size_t GetSizeOfArtifactsFor(const GURL& url);
-  bool GetCreatedTime(const GURL& url, base::Time* created_time);
-  bool GetLastModifiedTime(const GURL& url, base::Time* last_modified_time);
+  size_t GetSizeOfArtifacts(const DirectoryKey& key);
+  bool GetCreatedTime(const DirectoryKey& key, base::Time* created_time);
+  bool GetLastModifiedTime(const DirectoryKey& key,
+                           base::Time* last_modified_time);
+
+  // Returns true if the directory for |key| exists.
+  bool DirectoryExists(const DirectoryKey& key);
 
-  // Creates or gets a subdirectory under |root_directory|/ for |url| and
+  // Creates or gets a subdirectory under |root_directory| for |key| and
   // assigns it to |directory|. Returns true on success. If the directory was
-  // compressed then it is uncompressed automatically.
-  bool CreateOrGetDirectoryFor(const GURL& url, base::FilePath* directory);
+  // compressed then it will be uncompressed automatically.
+  bool CreateOrGetDirectory(const DirectoryKey& key, base::FilePath* directory);
 
   // Compresses the directory associated with |url|. Returns true on success or
   // if the directory was already compressed.
   // NOTE: an empty directory or a directory containing only empty
-  // files/directories will not compress.
-  bool CompressDirectoryFor(const GURL& url);
+  // files/directories will not be compressed.
+  bool CompressDirectory(const DirectoryKey& key);
 
-  // Deletes artifacts associated with |urls|.
-  void DeleteArtifactsFor(const std::vector<GURL>& urls);
+  // Deletes artifacts associated with |key| or |keys|.
+  void DeleteArtifacts(const DirectoryKey& key);
+  void DeleteArtifacts(const std::vector<DirectoryKey>& keys);
 
-  // Deletes all stored paint previews stored in the |profile_directory_|.
+  // Deletes all stored paint previews stored in the |root_directory_|.
   void DeleteAll();
 
  private:
@@ -50,12 +70,9 @@ class FileManager {
     kZip = 2,
   };
 
-  StorageType GetPathForUrl(const GURL& url, base::FilePath* path);
+  StorageType GetPathForKey(const DirectoryKey& key, base::FilePath* path);
 
   base::FilePath root_directory_;
-
-  FileManager(const FileManager&) = delete;
-  FileManager& operator=(const FileManager&) = delete;
 };
 
 }  // namespace paint_preview

components/paint_preview/browser/file_manager_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 namespace paint_preview {
 
@@ -17,20 +18,22 @@ TEST(FileManagerTest, TestStats) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   FileManager manager(temp_dir.GetPath());
-  GURL url("https://www.chromium.org");
-  GURL missing_url("https://www.muimorhc.org");
+  auto valid_key = manager.CreateKey(GURL("https://www.chromium.org"));
+  auto missing_key = manager.CreateKey(GURL("https://www.muimorhc.org"));
   base::FilePath directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(valid_key, &directory));
   EXPECT_FALSE(directory.empty());
+  EXPECT_TRUE(manager.DirectoryExists(valid_key));
+  EXPECT_FALSE(manager.DirectoryExists(missing_key));
 
   base::Time created_time;
-  EXPECT_FALSE(manager.GetCreatedTime(missing_url, &created_time));
-  EXPECT_TRUE(manager.GetCreatedTime(url, &created_time));
+  EXPECT_FALSE(manager.GetCreatedTime(missing_key, &created_time));
+  EXPECT_TRUE(manager.GetCreatedTime(valid_key, &created_time));
 
   base::TouchFile(directory, now - base::TimeDelta::FromSeconds(1),
                   now - base::TimeDelta::FromSeconds(1));
   base::Time accessed_time;
-  EXPECT_TRUE(manager.GetLastModifiedTime(url, &accessed_time));
+  EXPECT_TRUE(manager.GetLastModifiedTime(valid_key, &accessed_time));
   base::FilePath proto_path = directory.AppendASCII("paint_preview.pb");
   base::File file(proto_path,
                   base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
@@ -41,28 +44,28 @@ TEST(FileManagerTest, TestStats) {
   base::TouchFile(directory, now + base::TimeDelta::FromSeconds(1),
                   now + base::TimeDelta::FromSeconds(1));
   base::Time later_accessed_time;
-  EXPECT_FALSE(manager.GetLastModifiedTime(missing_url, &created_time));
-  EXPECT_TRUE(manager.GetLastModifiedTime(url, &later_accessed_time));
+  EXPECT_FALSE(manager.GetLastModifiedTime(missing_key, &created_time));
+  EXPECT_TRUE(manager.GetLastModifiedTime(valid_key, &later_accessed_time));
   EXPECT_GT(later_accessed_time, accessed_time);
 
-  EXPECT_GE(manager.GetSizeOfArtifactsFor(url), kSize);
+  EXPECT_GE(manager.GetSizeOfArtifacts(valid_key), kSize);
 }
 
 TEST(FileManagerTest, TestCreateOrGetDirectory) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   FileManager manager(temp_dir.GetPath());
-  GURL url("https://www.chromium.org");
+  auto key = manager.CreateKey(1U);
 
   // Create a new directory.
   base::FilePath new_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &new_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(key, &new_directory));
   EXPECT_FALSE(new_directory.empty());
   EXPECT_TRUE(base::PathExists(new_directory));
 
   // Open an existing directory.
   base::FilePath existing_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &existing_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(key, &existing_directory));
   EXPECT_FALSE(existing_directory.empty());
   EXPECT_EQ(existing_directory, new_directory);
   EXPECT_TRUE(base::PathExists(existing_directory));
@@ -86,16 +89,16 @@ TEST(FileManagerTest, TestCreateOrGetDirectory) {
 
   // Compress.
   base::FilePath zip_path = existing_directory.AddExtensionASCII(".zip");
-  EXPECT_TRUE(manager.CompressDirectoryFor(url));
+  EXPECT_TRUE(manager.CompressDirectory(key));
   EXPECT_FALSE(base::PathExists(existing_directory));
   EXPECT_FALSE(base::PathExists(test_file_path));
   EXPECT_FALSE(base::PathExists(test_file_path_empty));
   EXPECT_TRUE(base::PathExists(zip_path));
-  EXPECT_GT(manager.GetSizeOfArtifactsFor(url), 0U);
+  EXPECT_GT(manager.GetSizeOfArtifacts(key), 0U);
   existing_directory.clear();
 
   // Open a compressed file.
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &existing_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(key, &existing_directory));
   EXPECT_EQ(existing_directory, new_directory);
   EXPECT_TRUE(base::PathExists(existing_directory));
   EXPECT_TRUE(base::PathExists(test_file_path));
@@ -107,16 +110,16 @@ TEST(FileManagerTest, TestCompressDirectory) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   FileManager manager(temp_dir.GetPath());
-  GURL url("https://www.chromium.org");
+  auto key = manager.CreateKey(GURL("https://www.chromium.org"));
 
   base::FilePath new_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &new_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(key, &new_directory));
   EXPECT_FALSE(new_directory.empty());
   EXPECT_TRUE(base::PathExists(new_directory));
 
   // Compression fails without valid contents.
   base::FilePath zip_path = new_directory.AddExtensionASCII(".zip");
-  EXPECT_FALSE(manager.CompressDirectoryFor(url));
+  EXPECT_FALSE(manager.CompressDirectory(key));
   EXPECT_TRUE(base::PathExists(new_directory));
   EXPECT_FALSE(base::PathExists(zip_path));
 
@@ -128,7 +131,7 @@ TEST(FileManagerTest, TestCompressDirectory) {
     file.WriteAtCurrentPos(data.data(), data.size());
   }
 
-  EXPECT_TRUE(manager.CompressDirectoryFor(url));
+  EXPECT_TRUE(manager.CompressDirectory(key));
   EXPECT_FALSE(base::PathExists(new_directory));
   EXPECT_TRUE(base::PathExists(zip_path));
 }
@@ -138,29 +141,29 @@ TEST(FileManagerTest, TestDeleteArtifactsFor) {
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   FileManager manager(temp_dir.GetPath());
 
-  GURL cr_url("https://www.chromium.org");
+  auto cr_key = manager.CreateKey(GURL("https://www.chromium.org"));
   base::FilePath cr_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(cr_url, &cr_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(cr_key, &cr_directory));
   EXPECT_FALSE(cr_directory.empty());
   EXPECT_TRUE(base::PathExists(cr_directory));
 
-  GURL w3_url("https://www.w3.org");
+  auto w3_key = manager.CreateKey(GURL("https://www.w3.org"));
   base::FilePath w3_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(w3_url, &w3_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(w3_key, &w3_directory));
   EXPECT_FALSE(w3_directory.empty());
   EXPECT_TRUE(base::PathExists(w3_directory));
 
-  manager.DeleteArtifactsFor(std::vector<GURL>({cr_url}));
+  manager.DeleteArtifacts(cr_key);
   EXPECT_FALSE(base::PathExists(cr_directory));
   EXPECT_TRUE(base::PathExists(w3_directory));
 
   base::FilePath new_cr_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(cr_url, &new_cr_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(cr_key, &new_cr_directory));
   EXPECT_FALSE(new_cr_directory.empty());
   EXPECT_TRUE(base::PathExists(new_cr_directory));
   EXPECT_EQ(cr_directory, new_cr_directory);
 
-  manager.DeleteArtifactsFor(std::vector<GURL>({cr_url, w3_url}));
+  manager.DeleteArtifacts(std::vector<DirectoryKey>({cr_key, w3_key}));
   EXPECT_FALSE(base::PathExists(new_cr_directory));
   EXPECT_FALSE(base::PathExists(w3_directory));
 }
@@ -169,14 +172,14 @@ TEST(FileManagerTest, TestDeleteAll) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   FileManager manager(temp_dir.GetPath());
-  GURL cr_url("https://www.chromium.org");
+  auto cr_key = manager.CreateKey(GURL("https://www.chromium.org"));
   base::FilePath cr_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(cr_url, &cr_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(cr_key, &cr_directory));
   EXPECT_FALSE(cr_directory.empty());
   EXPECT_TRUE(base::PathExists(cr_directory));
-  GURL w3_url("https://www.w3.org");
+  auto w3_key = manager.CreateKey(GURL("https://www.w3.org"));
   base::FilePath w3_directory;
-  EXPECT_TRUE(manager.CreateOrGetDirectoryFor(w3_url, &w3_directory));
+  EXPECT_TRUE(manager.CreateOrGetDirectory(w3_key, &w3_directory));
   EXPECT_FALSE(w3_directory.empty());
   EXPECT_TRUE(base::PathExists(w3_directory));
   manager.DeleteAll();

components/paint_preview/browser/paint_preview_base_service_unittest.cc

@@ -160,8 +160,9 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) {
   auto* service = GetService();
   EXPECT_FALSE(service->IsOffTheRecord());
   base::FilePath path;
-  ASSERT_TRUE(service->GetFileManager()->CreateOrGetDirectoryFor(
-      web_contents()->GetLastCommittedURL(), &path));
+  auto* manager = service->GetFileManager();
+  ASSERT_TRUE(manager->CreateOrGetDirectory(
+      manager->CreateKey(web_contents()->GetLastCommittedURL()), &path));
 
   base::RunLoop loop;
   service->CapturePaintPreview(
@@ -212,8 +213,9 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) {
   auto* service = GetService();
   EXPECT_FALSE(service->IsOffTheRecord());
   base::FilePath path;
-  ASSERT_TRUE(service->GetFileManager()->CreateOrGetDirectoryFor(
-      web_contents()->GetLastCommittedURL(), &path));
+  auto* manager = service->GetFileManager();
+  ASSERT_TRUE(manager->CreateOrGetDirectory(
+      manager->CreateKey(web_contents()->GetLastCommittedURL()), &path));
 
   base::RunLoop loop;
   service->CapturePaintPreview(
@@ -246,8 +248,9 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) {
   auto* service = GetServiceWithRejectionPolicy();
   EXPECT_FALSE(service->IsOffTheRecord());
   base::FilePath path;
-  ASSERT_TRUE(service->GetFileManager()->CreateOrGetDirectoryFor(
-      web_contents()->GetLastCommittedURL(), &path));
+  auto* manager = service->GetFileManager();
+  ASSERT_TRUE(manager->CreateOrGetDirectory(
+      manager->CreateKey(web_contents()->GetLastCommittedURL()), &path));
 
   base::RunLoop loop;
   service->CapturePaintPreview(