FS: Notify of changes to intermediate directories for CreateDirectory

ObfuscatedFileUtil::CreateDirectory has a `recursive` flag which
mimics the behavior of `mkdir -p` by creating intermediate directories,
if necessary. However, this incorrectly reported all changes as
occurring on the target FileSystemURL.

For example, given foo/, the following changes would be signaled when
recursively creating directory foo/bar/baz/

Before:
- OnCreateDirectory on foo/bar/baz/
- OnCreateDirectory on foo/bar/baz/

After:
- OnCreateDirectory on foo/bar/
- OnCreateDirectory on foo/bar/baz/

Bug: 1019297, 1486978
Change-Id: Ib5aa74fa5c23d596c14b038d058b2c7c1b61a25f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4895368
Auto-Submit: Austin Sullivan <asully@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1205376}

storage/browser/file_system/obfuscated_file_util.cc

@@ -456,8 +456,21 @@ base::File::Error ObfuscatedFileUtil::CreateDirectory(
     if (error != base::File::FILE_OK)
       return error;
     UpdateUsage(context, url, growth);
+
+    // Appropriately report changes when recursively creating a directory by
+    // constructing the FileSystemURL of created intermediate directories.
+    base::FilePath changed_path = url.virtual_path();
+    for (size_t i = components.size() - 1; i > index; --i) {
+      changed_path = VirtualPath::DirName(changed_path);
+    }
+    auto created_directory_url =
+        context->file_system_context()->CreateCrackedFileSystemURL(
+            url.storage_key(), url.mount_type(), changed_path);
+    if (url.bucket().has_value()) {
+      created_directory_url.SetBucket(url.bucket().value());
+    }
     context->change_observers()->Notify(&FileChangeObserver::OnCreateDirectory,
-                                        url);
+                                        created_directory_url);
     if (first) {
       first = false;
       TouchDirectory(db, file_info.parent_id);

storage/browser/file_system/obfuscated_file_util_unittest.cc

@@ -56,6 +56,7 @@
 #include "storage/browser/test/test_file_system_context.h"
 #include "storage/common/database/database_identifier.h"
 #include "storage/common/file_system/file_system_types.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/storage_key/storage_key.h"
 #include "third_party/leveldatabase/leveldb_chrome.h"
@@ -1014,12 +1015,18 @@ TEST_P(ObfuscatedFileUtilTest, TestCreateAndDeleteFile) {
   context = NewContext(nullptr);
   bool exclusive = true;
   bool recursive = true;
-  FileSystemURL directory_url = CreateURLFromUTF8("series/of/directories");
+  FileSystemURL root_url = CreateURLFromUTF8("series");
+  FileSystemURL intermediate_url = FileSystemURLAppendUTF8(root_url, "of");
+  FileSystemURL directory_url =
+      FileSystemURLAppendUTF8(intermediate_url, "directories");
   url = FileSystemURLAppendUTF8(directory_url, "file name");
   EXPECT_EQ(base::File::FILE_OK,
             ofu()->CreateDirectory(context.get(), directory_url, exclusive,
                                    recursive));
-  // The oepration created 3 directories recursively.
+  // The operation created 3 directories recursively.
+  EXPECT_THAT(
+      change_observer()->get_changed_urls(),
+      testing::UnorderedElementsAre(root_url, intermediate_url, directory_url));
   EXPECT_EQ(3, change_observer()->get_and_reset_create_directory_count());
 
   context = NewContext(nullptr);

storage/browser/test/mock_file_change_observer.cc

@@ -27,27 +27,34 @@ ChangeObserverList MockFileChangeObserver::CreateList(
 
 void MockFileChangeObserver::OnCreateFile(const FileSystemURL& url) {
   create_file_count_++;
+  changed_urls_.insert(url);
 }
 
 void MockFileChangeObserver::OnCreateFileFrom(const FileSystemURL& url,
                                               const FileSystemURL& src) {
   create_file_from_count_++;
+  changed_urls_.insert(url);
+  changed_urls_.insert(src);
 }
 
 void MockFileChangeObserver::OnRemoveFile(const FileSystemURL& url) {
   remove_file_count_++;
+  changed_urls_.insert(url);
 }
 
 void MockFileChangeObserver::OnModifyFile(const FileSystemURL& url) {
   modify_file_count_++;
+  changed_urls_.insert(url);
 }
 
 void MockFileChangeObserver::OnCreateDirectory(const FileSystemURL& url) {
   create_directory_count_++;
+  changed_urls_.insert(url);
 }
 
 void MockFileChangeObserver::OnRemoveDirectory(const FileSystemURL& url) {
   remove_directory_count_++;
+  changed_urls_.insert(url);
 }
 
 }  // namespace storage

storage/browser/test/mock_file_change_observer.h

@@ -41,17 +41,19 @@ class MockFileChangeObserver : public FileChangeObserver {
     modify_file_count_ = 0;
     create_directory_count_ = 0;
     remove_directory_count_ = 0;
+    changed_urls_.clear();
   }
 
   bool HasNoChange() const {
-    return create_file_count_ == 0 &&
+    bool has_change = create_file_count_ || create_file_from_count_ ||
-           create_file_from_count_ == 0 &&
+                      remove_file_count_ || modify_file_count_ ||
-           remove_file_count_ == 0 &&
+                      create_directory_count_ || remove_directory_count_;
-           modify_file_count_ == 0 &&
+    CHECK_NE(has_change, changed_urls_.empty());
-           create_directory_count_ == 0 &&
+    return !has_change;
-           remove_directory_count_ == 0;
   }
 
+  const FileSystemURLSet& get_changed_urls() const { return changed_urls_; }
+
   int create_file_count() const { return create_file_count_; }
   int create_file_from_count() const { return create_file_from_count_; }
   int remove_file_count() const { return remove_file_count_; }
@@ -62,35 +64,43 @@ class MockFileChangeObserver : public FileChangeObserver {
   int get_and_reset_create_file_count() {
     int tmp = create_file_count_;
     create_file_count_ = 0;
+    changed_urls_.clear();
     return tmp;
   }
   int get_and_reset_create_file_from_count() {
     int tmp = create_file_from_count_;
     create_file_from_count_ = 0;
+    changed_urls_.clear();
     return tmp;
   }
   int get_and_reset_remove_file_count() {
     int tmp = remove_file_count_;
     remove_file_count_ = 0;
+    changed_urls_.clear();
     return tmp;
   }
   int get_and_reset_modify_file_count() {
     int tmp = modify_file_count_;
     modify_file_count_ = 0;
+    changed_urls_.clear();
     return tmp;
   }
   int get_and_reset_create_directory_count() {
     int tmp = create_directory_count_;
     create_directory_count_ = 0;
+    changed_urls_.clear();
     return tmp;
   }
   int get_and_reset_remove_directory_count() {
     int tmp = remove_directory_count_;
     remove_directory_count_ = 0;
+    changed_urls_.clear();
     return tmp;
   }
 
  private:
+  FileSystemURLSet changed_urls_;
+
   int create_file_count_;
   int create_file_from_count_;
   int remove_file_count_;