Convert visitedlink to use mojo.

components/visitedlink/browser/visitedlink_master.cc

 // Copyright (c) 2012 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 "components/visitedlink/browser/visitedlink_master.h"
 
 #include <stdio.h>
 #include <string.h>
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/containers/stack_container.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/rand_util.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 #include "components/visitedlink/browser/visitedlink_delegate.h"
 #include "components/visitedlink/browser/visitedlink_event_listener.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
@@ skipping 26 matching lines @@
 const unsigned VisitedLinkMaster::kDefaultTableSize = 16381;
 
 const size_t VisitedLinkMaster::kBigDeleteThreshold = 64;
 
 namespace {
 
 // Fills the given salt structure with some quasi-random values
 // It is not necessary to generate a cryptographically strong random string,
 // only that it be reasonably different for different users.
 void GenerateSalt(uint8_t salt[LINK_SALT_LENGTH]) {
-  DCHECK_EQ(LINK_SALT_LENGTH, 8) << "This code assumes the length of the salt";
+  static_assert(LINK_SALT_LENGTH == 8,
+                "This code assumes the length of the salt");
   uint64_t randval = base::RandUint64();
   memcpy(salt, &randval, 8);
 }
 
 // Opens file on a background thread to not block UI thread.
 void AsyncOpen(FILE** file, const base::FilePath& filename) {
   *file = base::OpenFile(filename, "wb+");
   DLOG_IF(ERROR, !(*file)) << "Failed to open file " << filename.value();
 }
 
@@ skipping 40 matching lines @@
   if (*file)
     base::IgnoreResult(fclose(*file));
   free(file);
 }
 
 }  // namespace
 
 struct VisitedLinkMaster::LoadFromFileResult
     : public base::RefCountedThreadSafe<LoadFromFileResult> {
   LoadFromFileResult(base::ScopedFILE file,
-                     std::unique_ptr<base::SharedMemory> shared_memory,
-                     Fingerprint* hash_table,
+                     mojo::ScopedSharedBufferHandle shared_memory,
+                     mojo::ScopedSharedBufferMapping hash_table,
                      int32_t num_entries,
                      int32_t used_count,
                      uint8_t salt[LINK_SALT_LENGTH]);
 
   base::ScopedFILE file;
-  std::unique_ptr<base::SharedMemory> shared_memory;
-  Fingerprint* hash_table;
+  mojo::ScopedSharedBufferHandle shared_memory;
+  mojo::ScopedSharedBufferMapping hash_table;
   int32_t num_entries;
   int32_t used_count;
   uint8_t salt[LINK_SALT_LENGTH];
 
  private:
   friend class base::RefCountedThreadSafe<LoadFromFileResult>;
   virtual ~LoadFromFileResult();
 
   DISALLOW_COPY_AND_ASSIGN(LoadFromFileResult);
 };
 
 VisitedLinkMaster::LoadFromFileResult::LoadFromFileResult(
     base::ScopedFILE file,
-    std::unique_ptr<base::SharedMemory> shared_memory,
-    Fingerprint* hash_table,
+    mojo::ScopedSharedBufferHandle shared_memory,
+    mojo::ScopedSharedBufferMapping hash_table,
     int32_t num_entries,
     int32_t used_count,
     uint8_t salt[LINK_SALT_LENGTH])
     : file(std::move(file)),
       shared_memory(std::move(shared_memory)),
-      hash_table(hash_table),
+      hash_table(std::move(hash_table)),
       num_entries(num_entries),
       used_count(used_count) {
   memcpy(this->salt, salt, LINK_SALT_LENGTH);
 }
 
 VisitedLinkMaster::LoadFromFileResult::~LoadFromFileResult() {
 }
 
 // TableBuilder ---------------------------------------------------------------
 
@@ skipping 48 matching lines @@
   // Stores the fingerprints we computed on the background thread.
   VisitedLinkCommon::Fingerprints fingerprints_;
 
   DISALLOW_COPY_AND_ASSIGN(TableBuilder);
 };
 
 // VisitedLinkMaster ----------------------------------------------------------
 
 VisitedLinkMaster::VisitedLinkMaster(content::BrowserContext* browser_context,
                                      VisitedLinkDelegate* delegate,
-                                     bool persist_to_disk)
+                                     bool persist_to_disk,
+                                     std::unique_ptr<Listener> listener)
     : browser_context_(browser_context),
       delegate_(delegate),
-      listener_(new VisitedLinkEventListener(this, browser_context)),
+      listener_(
+          listener ? std::move(listener)
+                   : base::MakeUnique<VisitedLinkEventListener>(browser_context,
+                                                                nullptr)),
       persist_to_disk_(persist_to_disk),
       table_is_loading_from_file_(false),
       weak_ptr_factory_(this) {
   InitMembers();
 }
 
 VisitedLinkMaster::VisitedLinkMaster(Listener* listener,
                                      VisitedLinkDelegate* delegate,
                                      bool persist_to_disk,
                                      bool suppress_rebuild,
@@ skipping 32 matching lines @@
     // state. On the next start table will be rebuilt.
     base::FilePath filename;
     GetDatabaseFileName(&filename);
     PostIOTask(FROM_HERE,
                base::Bind(IgnoreResult(&base::DeleteFile), filename, false));
   }
 }
 
 void VisitedLinkMaster::InitMembers() {
   file_ = NULL;
-  shared_memory_ = NULL;
   shared_memory_serial_ = 0;
   used_items_ = 0;
   table_size_override_ = 0;
   suppress_rebuild_ = false;
   sequence_token_ = base::SequencedWorkerPool::GetSequenceToken();
 }
 
 bool VisitedLinkMaster::Init() {
   // Create the temporary table. If the table is rebuilt that temporary table
   // will be became the main table.
   // The salt must be generated before the table so that it can be copied to
   // the shared memory.
   GenerateSalt(salt_);
   if (!CreateURLTable(DefaultTableSize()))
     return false;
 
+  if (shared_memory_.is_valid())

[brettw, 2016/09/14 23:02:13]

I don't understand this addition, can you describe what's changed to now require
this?

[Sam McNally, 2016/09/15 04:12:16]

Previously, VisitedLinkEventListener received a pointer to its
VisitedLinkMaster, which it only used to get the shared memory from the master
(to send to newly launched renderer processes).

I wanted to break this dependency so I could inject a VisitedLinkEventListener
with a mock Timer in tests. Now, the listener holds a handle to the shared
memory which it replaces on each NewTable() notification. That maintains the
existing behaviour, except for before the first NewTable() notification. To fix
this discrepancy, this pushes the initial shared memory to the listener once
it's ready.

+    listener_->NewTable(shared_memory_.get());
+
 #ifndef NDEBUG
   DebugValidate();
 #endif
 
   if (persist_to_disk_) {
     if (InitFromFile())
       return true;
   }
   return InitFromScratch(suppress_rebuild_);
 }
@@ skipping 348 matching lines @@
   base::ScopedFILE file_closer(base::OpenFile(filename, "rb+"));
   if (!file_closer.get())
     return false;
 
   int32_t num_entries, used_count;
   uint8_t salt[LINK_SALT_LENGTH];
   if (!ReadFileHeader(file_closer.get(), &num_entries, &used_count, salt))
     return false;  // Header isn't valid.
 
   // Allocate and read the table.
-  std::unique_ptr<base::SharedMemory> shared_memory;
-  VisitedLinkCommon::Fingerprint* hash_table;
+  mojo::ScopedSharedBufferHandle shared_memory;
+  mojo::ScopedSharedBufferMapping hash_table;
   if (!CreateApartURLTable(num_entries, salt, &shared_memory, &hash_table))
     return false;
 
-  if (!ReadFromFile(file_closer.get(), kFileHeaderSize, hash_table,
+  if (!ReadFromFile(file_closer.get(), kFileHeaderSize, hash_table.get(),
                     num_entries * sizeof(Fingerprint))) {
     return false;
   }
 
-  *load_from_file_result =
-      new LoadFromFileResult(std::move(file_closer), std::move(shared_memory),
-                             hash_table, num_entries, used_count, salt);
+  *load_from_file_result = new LoadFromFileResult(
+      std::move(file_closer), std::move(shared_memory), std::move(hash_table),
+      num_entries, used_count, salt);
   return true;
 }
 
 void VisitedLinkMaster::OnTableLoadComplete(
     bool success,
     scoped_refptr<LoadFromFileResult> load_from_file_result) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(persist_to_disk_);
   DCHECK(!table_builder_.get());
 
@@ skipping 20 matching lines @@
     return;
   }
 
   // This temporary sets are needed only to rebuild table.
   added_since_rebuild_.clear();
   deleted_since_rebuild_.clear();
 
   DCHECK(load_from_file_result.get());
 
   // Delete the previous table.
-  DCHECK(shared_memory_);
-  delete shared_memory_;
-  shared_memory_ = nullptr;
+  DCHECK(shared_memory_.is_valid());
+  shared_memory_.reset();
+  hash_table_mapping_.reset();
 
   // Assign the open file.
   DCHECK(!file_);
   DCHECK(load_from_file_result->file.get());
   file_ = static_cast<FILE**>(malloc(sizeof(*file_)));
   *file_ = load_from_file_result->file.release();
 
   // Assign the loaded table.
-  DCHECK(load_from_file_result->shared_memory.get());
+  DCHECK(load_from_file_result->shared_memory.is_valid());
   DCHECK(load_from_file_result->hash_table);
   memcpy(salt_, load_from_file_result->salt, LINK_SALT_LENGTH);
-  shared_memory_ = load_from_file_result->shared_memory.release();
-  hash_table_ = load_from_file_result->hash_table;
+  shared_memory_ = std::move(load_from_file_result->shared_memory);
+  hash_table_mapping_ = std::move(load_from_file_result->hash_table);
+  hash_table_ = static_cast<Fingerprint*>(hash_table_mapping_.get());
   table_length_ = load_from_file_result->num_entries;
   used_items_ = load_from_file_result->used_count;
 
 #ifndef NDEBUG
   DebugValidate();
 #endif
 
   // Send an update notification to all child processes.
-  listener_->NewTable(shared_memory_);
+  listener_->NewTable(shared_memory_.get());
 
   if (!added_since_load_.empty() || !deleted_since_load_.empty()) {
     // Resize the table if the table doesn't have enough capacity.
     int new_used_items =
         used_items_ + static_cast<int>(added_since_load_.size());
     if (new_used_items >= table_length_)
       ResizeTable(NewTableSizeForCount(new_used_items));
 
     // Also add anything that was added while we were asynchronously
     // loading the table.
@@ skipping 99 matching lines @@
     return false;
 
   base::FilePath profile_dir = browser_context_->GetPath();
   *filename = profile_dir.Append(FILE_PATH_LITERAL("Visited Links"));
   return true;
 }
 
 // Initializes the shared memory structure. The salt should already be filled
 // in so that it can be written to the shared memory
 bool VisitedLinkMaster::CreateURLTable(int32_t num_entries) {
-  std::unique_ptr<base::SharedMemory> shared_memory;
-  VisitedLinkCommon::Fingerprint* hash_table;
+  mojo::ScopedSharedBufferHandle shared_memory;
+  mojo::ScopedSharedBufferMapping hash_table;
   if (CreateApartURLTable(num_entries, salt_, &shared_memory, &hash_table)) {
-    shared_memory_ = shared_memory.release();
-    hash_table_ = hash_table;
+    shared_memory_ = std::move(shared_memory);
+    hash_table_mapping_ = std::move(hash_table);
+    hash_table_ = static_cast<Fingerprint*>(hash_table_mapping_.get());
     table_length_ = num_entries;
     used_items_ = 0;
     return true;
   }
 
   return false;
 }
 
 // static
 bool VisitedLinkMaster::CreateApartURLTable(
     int32_t num_entries,
     const uint8_t salt[LINK_SALT_LENGTH],
-    std::unique_ptr<base::SharedMemory>* shared_memory,
-    VisitedLinkCommon::Fingerprint** hash_table) {
+    mojo::ScopedSharedBufferHandle* shared_memory,
+    mojo::ScopedSharedBufferMapping* hash_table) {
   DCHECK(salt);
   DCHECK(shared_memory);
   DCHECK(hash_table);
 
   // The table is the size of the table followed by the entries.
   uint32_t alloc_size =
       num_entries * sizeof(Fingerprint) + sizeof(SharedHeader);
 
   // Create the shared memory object.
-  std::unique_ptr<base::SharedMemory> sh_mem(new base::SharedMemory());
-  if (!sh_mem)
+  mojo::ScopedSharedBufferHandle shared_buffer =
+      mojo::SharedBufferHandle::Create(alloc_size);
+  if (!shared_buffer.is_valid())
+    return false;
+  {
+    mojo::ScopedSharedBufferMapping mapping = shared_buffer->Map(alloc_size);
+    if (!mapping)
+      return false;
+
+    memset(mapping.get(), 0, alloc_size);
+
+    // Save the header for other processes to read.
+    SharedHeader* header = static_cast<SharedHeader*>(mapping.get());
+    header->length = num_entries;
+    memcpy(header->salt, salt, LINK_SALT_LENGTH);
+  }
+
+  // Our table pointer is just the data immediately following the header.
+  mojo::ScopedSharedBufferMapping hash_table_mapping =

[brettw, 2016/09/14 23:02:13]

I kind of preferred the old way. Unmapping and remapping the entire table are
heavyweight VM operations just to move a pointer that the old code already
handled. Are there benefits of doing it this way?

[Sam McNally, 2016/09/15 04:12:15]

I think it makes the code a bit simpler, but adding a helper to convert from the
mapping to the hash table seems fine (and moves the casts to a single place,
which is nice).

+      shared_buffer->MapAtOffset(num_entries * sizeof(Fingerprint),
+                                 sizeof(SharedHeader));
+  if (!hash_table_mapping)
     return false;
 
-  base::SharedMemoryCreateOptions options;
-  options.size = alloc_size;
-  options.share_read_only = true;
-
-  if (!sh_mem->Create(options) || !sh_mem->Map(alloc_size))
-    return false;
-
-  memset(sh_mem->memory(), 0, alloc_size);
-
-  // Save the header for other processes to read.
-  SharedHeader* header = static_cast<SharedHeader*>(sh_mem->memory());
-  header->length = num_entries;
-  memcpy(header->salt, salt, LINK_SALT_LENGTH);
-
-  // Our table pointer is just the data immediately following the size.
-  *hash_table = reinterpret_cast<Fingerprint*>(
-      static_cast<char*>(sh_mem->memory()) + sizeof(SharedHeader));
-
-  *shared_memory = std::move(sh_mem);
+  *shared_memory = std::move(shared_buffer);
+  *hash_table = std::move(hash_table_mapping);
 
   return true;
 }
 
 bool VisitedLinkMaster::BeginReplaceURLTable(int32_t num_entries) {
-  base::SharedMemory *old_shared_memory = shared_memory_;
-  Fingerprint* old_hash_table = hash_table_;
+  mojo::ScopedSharedBufferHandle old_shared_memory = std::move(shared_memory_);
+  mojo::ScopedSharedBufferMapping old_hash_table_mapping =
+      std::move(hash_table_mapping_);
   int32_t old_table_length = table_length_;
   if (!CreateURLTable(num_entries)) {
     // Try to put back the old state.
-    shared_memory_ = old_shared_memory;
-    hash_table_ = old_hash_table;
+    shared_memory_ = std::move(old_shared_memory);
+    hash_table_mapping_ = std::move(old_hash_table_mapping);
+    hash_table_ = static_cast<Fingerprint*>(hash_table_mapping_.get());
     table_length_ = old_table_length;
     return false;
   }
 
 #ifndef NDEBUG
   DebugValidate();
 #endif
 
   return true;
 }
 
 void VisitedLinkMaster::FreeURLTable() {
-  if (shared_memory_) {
-    delete shared_memory_;
-    shared_memory_ = NULL;
-  }
+  shared_memory_.reset();
+  hash_table_mapping_.reset();
   if (!persist_to_disk_ || !file_)
     return;
   PostIOTask(FROM_HERE, base::Bind(&AsyncClose, file_));
   // AsyncClose() will close the file and free the memory pointed by |file_|.
   file_ = NULL;
 }
 
 bool VisitedLinkMaster::ResizeTableIfNecessary() {
   DCHECK(table_length_ > 0) << "Must have a table";
 
@@ skipping 12 matching lines @@
 
   // Table needs to grow or shrink.
   int new_size = NewTableSizeForCount(used_items_);
   DCHECK(new_size > used_items_);
   DCHECK(load <= min_table_load || new_size > table_length_);
   ResizeTable(new_size);
   return true;
 }
 
 void VisitedLinkMaster::ResizeTable(int32_t new_size) {
-  DCHECK(shared_memory_ && shared_memory_->memory() && hash_table_);
+  DCHECK(shared_memory_.is_valid() && hash_table_mapping_);
   shared_memory_serial_++;
 
 #ifndef NDEBUG
   DebugValidate();
 #endif
 
-  base::SharedMemory* old_shared_memory = shared_memory_;
-  Fingerprint* old_hash_table = hash_table_;
+  mojo::ScopedSharedBufferMapping old_hash_table_mapping =
+      std::move(hash_table_mapping_);
   int32_t old_table_length = table_length_;
-  if (!BeginReplaceURLTable(new_size))
+  if (!BeginReplaceURLTable(new_size)) {
+    hash_table_mapping_ = std::move(old_hash_table_mapping);
+    hash_table_ = static_cast<Fingerprint*>(hash_table_mapping_.get());
     return;
-
-  // Now we have two tables, our local copy which is the old one, and the new
-  // one loaded into this object where we need to copy the data.
-  for (int32_t i = 0; i < old_table_length; i++) {
-    Fingerprint cur = old_hash_table[i];
-    if (cur)
-      AddFingerprint(cur, false);
   }
 
-  // On error unmapping, just forget about it since we can't do anything
-  // else to release it.
-  delete old_shared_memory;
+  {
+    Fingerprint* old_hash_table =
+        static_cast<Fingerprint*>(old_hash_table_mapping.get());
+    // Now we have two tables, our local copy which is the old one, and the new
+    // one loaded into this object where we need to copy the data.
+    for (int32_t i = 0; i < old_table_length; i++) {
+      Fingerprint cur = old_hash_table[i];
+      if (cur)
+        AddFingerprint(cur, false);
+    }
+  }
+  old_hash_table_mapping.reset();
 
   // Send an update notification to all child processes so they read the new
   // table.
-  listener_->NewTable(shared_memory_);
+  listener_->NewTable(shared_memory_.get());
 
 #ifndef NDEBUG
   DebugValidate();
 #endif
 
   // The new table needs to be written to disk.
   if (persist_to_disk_)
     WriteFullTable();
 }
 
@@ skipping 47 matching lines @@
 }
 
 // See the TableBuilder declaration above for how this works.
 void VisitedLinkMaster::OnTableRebuildComplete(
     bool success,
     const std::vector<Fingerprint>& fingerprints) {
   if (success) {
     // Replace the old table with a new blank one.
     shared_memory_serial_++;
 
-    // We are responsible for freeing it AFTER it has been replaced if
-    // replacement succeeds.
-    base::SharedMemory* old_shared_memory = shared_memory_;
-
     int new_table_size = NewTableSizeForCount(
         static_cast<int>(fingerprints.size() + added_since_rebuild_.size()));
     if (BeginReplaceURLTable(new_table_size)) {
-      // Free the old table.
-      delete old_shared_memory;
-
       // Add the stored fingerprints to the hash table.
       for (const auto& fingerprint : fingerprints)
         AddFingerprint(fingerprint, false);
 
       // Also add anything that was added while we were asynchronously
       // generating the new table.
       for (const auto& fingerprint : added_since_rebuild_)
         AddFingerprint(fingerprint, false);
       added_since_rebuild_.clear();
 
       // Now handle deletions. Do not shrink the table now, we'll shrink it when
       // adding or deleting an url the next time.
       for (const auto& fingerprint : deleted_since_rebuild_)
         DeleteFingerprint(fingerprint, false);
       deleted_since_rebuild_.clear();
 
       // Send an update notification to all child processes.
-      listener_->NewTable(shared_memory_);
+      listener_->NewTable(shared_memory_.get());
       // All tabs which was loaded when table was being rebuilt
       // invalidate their links again.
       listener_->Reset(false);
 
       if (persist_to_disk_)
         WriteFullTable();
     }
   }
   table_builder_ = NULL;  // Will release our reference to the builder.
 
@@ skipping 89 matching lines @@
       BrowserThread::UI, FROM_HERE,
       base::Bind(&TableBuilder::OnCompleteMainThread, this));
 }
 
 void VisitedLinkMaster::TableBuilder::OnCompleteMainThread() {
   if (master_)
     master_->OnTableRebuildComplete(success_, fingerprints_);
 }
 
 }  // namespace visitedlink