browser_test that creates filesystem resources in extension origin

content/public/test/test_fileapi_operation_waiter.cc

Index: content/public/test/test_fileapi_operation_waiter.cc
diff --git a/content/public/test/test_fileapi_operation_waiter.cc b/content/public/test/test_fileapi_operation_waiter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0328eda7649be83f18a8e14380f8aeb38bbc0d1a
--- /dev/null
+++ b/content/public/test/test_fileapi_operation_waiter.cc
@@ -0,0 +1,114 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/public/test/test_fileapi_operation_waiter.h"
+
+#include "base/bind_helpers.h"
+#include "base/lazy_instance.h"
+#include "base/observer_list.h"
+#include "content/public/browser/browser_thread.h"
+#include "storage/browser/fileapi/file_system_context.h"
+#include "storage/browser/fileapi/sandbox_file_system_backend_delegate.h"
+
+namespace content {
+
+using storage::FileSystemContext;
+using storage::FileSystemURL;
+using storage::FileUpdateObserver;
+
+namespace {
+
+// Because of how fileapi internally creates copies of its observer lists,
+// removing an observer is not a supported operation. So to support temporary,

[alexmos, 2016/12/02 00:01:56]

That does sound like an awkward observer API.  So the regular
FileUpdateObservers (LocalFileChangeTracker, SandboxQuotaObserver, etc.) are
only destroyed on browser shutdown?  Your approach with the global observer
seems ok then.

[ncarter (slow), 2016/12/02 19:17:45]

From what I can tell, the other FileUpdateObservers haven't encountered this
problem because they're also owned by the FileSystemContext. There's one
FileSystemContext per StoragePartition, so to first approximation that means it
has a Profile lifetime.

I think it might be safe to bound the lifetime of this object by letting the
BrowserContext own it via SupportsUserData, but I don't want to overengineer it.
I somewhat doubt this observer will ever be used again.

FWIW: I wrote this six different ways, all of them different flavors of ugly,
before landing on the leaky singleton approach. The runner up was to use
refcounting, and AddRef at OnStartUpdate time, and Release at OnEndUpdate time
but it was ugly, depended strongly on the current impl, and there was still no
good way to remove an observer -- all you can really do is to restore the list
to what it was when you started observing, which means spinning up multiple
observers would have potentially very weird behavior.

+// test-style observers, we create one long-lived global observer instance that
+// dispatches to a list of short-lived observers.
+//
+// This object operates on the UI thread, though it registers itself as an
+// observer on the IO thread.
+class FileUpdateObserverMultiplexer : public FileUpdateObserver {
+ public:
+  FileUpdateObserverMultiplexer() {}
+
+  void AddObserver(FileSystemContext* context, FileUpdateObserver* observer) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+    // On first initialization, install ourself as an observer. We never
+    // uninstall, because we expect to leak.
+    if (!context_) {
+      // Currently we only listen to kFileSystemTypeTemporary; it should be fine
+      // to add other filesystem types as needed.
+      context_ = context;
+      base::Closure task = base::Bind(
+          &storage::SandboxFileSystemBackendDelegate::AddFileUpdateObserver,
+          base::Unretained(context_->sandbox_delegate()),
+          storage::kFileSystemTypeTemporary, base::Unretained(this),
+          base::RetainedRef(
+              BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)));
+      BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, task);
+    }
+
+    CHECK_EQ(context, context_) << "Multiprofile is not implemented";
+
+    observers_.AddObserver(observer);
+  }
+
+  void RemoveObserver(FileUpdateObserver* observer) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    observers_.RemoveObserver(observer);
+  }
+
+  // FileUpdateObserver overrides:
+  void OnStartUpdate(const FileSystemURL& url) override {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    for (auto& observer : observers_)
+      observer.OnStartUpdate(url);
+  }
+  void OnUpdate(const FileSystemURL& url, int64_t delta) override {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    for (auto& observer : observers_)
+      observer.OnUpdate(url, delta);
+  }
+  void OnEndUpdate(const FileSystemURL& url) override {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+

[alexmos, 2016/12/02 00:01:56]

nit: no blank line (for consistency with the other two)

+    for (auto& observer : observers_)
+      observer.OnEndUpdate(url);
+  }
+
+ private:
+  FileSystemContext* context_ = nullptr;
+  base::ObserverList<FileUpdateObserver> observers_;
+  DISALLOW_COPY_AND_ASSIGN(FileUpdateObserverMultiplexer);
+};
+
+static base::LazyInstance<FileUpdateObserverMultiplexer>::Leaky g_multiplexer =
+    LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+TestFileapiOperationWaiter::TestFileapiOperationWaiter(
+    FileSystemContext* context) {
+  g_multiplexer.Get().AddObserver(context, this);
+}
+
+TestFileapiOperationWaiter::~TestFileapiOperationWaiter() {
+  g_multiplexer.Get().RemoveObserver(this);
+}
+
+void TestFileapiOperationWaiter::WaitForEndUpdate() {
+  run_loop_.Run();
+}
+
+void TestFileapiOperationWaiter::OnStartUpdate(const FileSystemURL& url) {
+  did_start_update_ = true;
+}
+
+void TestFileapiOperationWaiter::OnUpdate(const FileSystemURL& url,
+                                          int64_t delta) {}
+
+void TestFileapiOperationWaiter::OnEndUpdate(const FileSystemURL& url) {
+  run_loop_.Quit();
+}
+
+}  // namespace content