Don't abuse LevelDBObserver interface to pass GetAll result.

content/renderer/dom_storage/local_storage_cached_area.cc

 // 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/renderer/dom_storage/local_storage_cached_area.h"
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/time/time.h"
 #include "content/common/dom_storage/dom_storage_map.h"
 #include "content/common/storage_partition_service.mojom.h"
 #include "content/renderer/dom_storage/local_storage_area.h"
 #include "content/renderer/dom_storage/local_storage_cached_areas.h"
+#include "mojo/public/cpp/bindings/strong_associated_binding.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/web/WebStorageEventDispatcher.h"
 
+namespace content {
+
 namespace {
 
 base::string16 Uint8VectorToString16(const std::vector<uint8_t>& input) {
   return base::string16(reinterpret_cast<const base::char16*>(input.data()),
                         input.size() / sizeof(base::char16));
 }
 
 std::vector<uint8_t> String16ToUint8Vector(const base::string16& input) {
   const uint8_t* data = reinterpret_cast<const uint8_t*>(input.data());
   return std::vector<uint8_t>(data, data + input.size() * sizeof(base::char16));
 }
 
+class GetAllCallback : public mojom::LevelDBWrapperGetAllCallback {
+ public:
+  static mojom::LevelDBWrapperGetAllCallbackAssociatedPtrInfo CreateAndBind(
+      mojo::AssociatedGroup* associated_group,
+      const base::Callback<void(bool)>& callback) {
+    mojom::LevelDBWrapperGetAllCallbackAssociatedPtrInfo ptr_info;
+    mojom::LevelDBWrapperGetAllCallbackAssociatedRequest request;
+    associated_group->CreateAssociatedInterface(
+        mojo::AssociatedGroup::WILL_PASS_PTR, &ptr_info, &request);
+    mojo::MakeStrongAssociatedBinding(
+        base::WrapUnique(new GetAllCallback(callback)), std::move(request));

[dcheng, 2016/12/22 07:21:47]

Nit: base::MakeUnique<GetAllCallback>(callback)

[Marijn Kruisselbrink, 2016/12/22 17:12:19]

That would require making the constructor public (which I could do of course,
but the current code seems just as valid style-wise to me).

+    return ptr_info;
+  }
+
+ private:
+  explicit GetAllCallback(const base::Callback<void(bool)>& callback)
+      : m_callback(callback) {}
+  void Complete(bool success) override { m_callback.Run(success); }
+
+  base::Callback<void(bool)> m_callback;
+};
+
 }  // namespace
 
-namespace content {
-
 // These methods are used to pack and unpack the page_url/storage_area_id into
 // source strings to/from the browser.
 std::string PackSource(const GURL& page_url,
                        const std::string& storage_area_id) {
   return page_url.spec() + "\n" + storage_area_id;
 }
 
 void UnpackSource(const std::string& source,
                   GURL* page_url,
                   std::string* storage_area_id) {
@@ skipping 164 matching lines @@
       }
       ++iter;
     }
   }
 
   blink::WebStorageEventDispatcher::dispatchLocalStorageEvent(
       base::NullableString16(), base::NullableString16(),
       base::NullableString16(), origin_.GetURL(), page_url, originating_area);
 }
 
-void LocalStorageCachedArea::GetAllComplete(const std::string& source) {
-  // Since the GetAll method is synchronous, we need this asynchronously
-  // delivered notification to avoid applying changes to the returned array
-  // that we already have.
-  if (source == get_all_request_id_) {
-    DCHECK(ignore_all_mutations_);
-    DCHECK(!get_all_request_id_.empty());
-    ignore_all_mutations_ = false;
-    get_all_request_id_.clear();
-  }
-}
-
 void LocalStorageCachedArea::KeyAddedOrChanged(
     const std::vector<uint8_t>& key,
     const std::vector<uint8_t>& new_value,
     const base::NullableString16& old_value,
     const std::string& source) {
   GURL page_url;
   std::string storage_area_id;
   UnpackSource(source, &page_url, &storage_area_id);
 
   base::string16 key_string = Uint8VectorToString16(key);
@@ skipping 22 matching lines @@
       key_string, old_value, new_value_string, origin_.GetURL(), page_url,
       originating_area);
 }
 
 void LocalStorageCachedArea::EnsureLoaded() {
   if (map_)
     return;
 
   base::TimeTicks before = base::TimeTicks::Now();
   ignore_all_mutations_ = true;
-  get_all_request_id_ = base::Uint64ToString(base::RandUint64());
   leveldb::mojom::DatabaseError status = leveldb::mojom::DatabaseError::OK;
   std::vector<content::mojom::KeyValuePtr> data;
-  leveldb_->GetAll(get_all_request_id_, &status, &data);
+  leveldb_->GetAll(GetAllCallback::CreateAndBind(
+                       leveldb_.associated_group(),
+                       base::Bind(&LocalStorageCachedArea::OnGetAllComplete,
+                                  weak_factory_.GetWeakPtr())),
+                   &status, &data);
 
   DOMStorageValuesMap values;
   for (size_t i = 0; i < data.size(); ++i) {
     values[Uint8VectorToString16(data[i]->key)] =
         base::NullableString16(Uint8VectorToString16(data[i]->value), false);
   }
 
   map_ = new DOMStorageMap(kPerStorageAreaQuota);
   map_->SwapValues(&values);
 
@@ skipping 40 matching lines @@
   if (--found->second == 0)
     ignore_key_mutations_.erase(found);
 }
 
 void LocalStorageCachedArea::OnClearComplete(bool success) {
   DCHECK(success);
   DCHECK(ignore_all_mutations_);
   ignore_all_mutations_ = false;
 }
 
+void LocalStorageCachedArea::OnGetAllComplete(bool success) {
+  // Since the GetAll method is synchronous, we need this asynchronously
+  // delivered notification to avoid applying changes to the returned array
+  // that we already have.
+  DCHECK(success);
+  DCHECK(ignore_all_mutations_);
+  ignore_all_mutations_ = false;
+}
+
 void LocalStorageCachedArea::Reset() {
   map_ = NULL;
   ignore_key_mutations_.clear();
   ignore_all_mutations_ = false;
   weak_factory_.InvalidateWeakPtrs();
 }
 
 }  // namespace content