Use a raw pointer instead of Singleton for PnaclHost and leak it.

components/nacl/browser/pnacl_host.cc

 // Copyright 2013 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/nacl/browser/pnacl_host.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/debug/leak_annotations.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/numerics/safe_math.h"
 #include "base/task_runner_util.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "components/nacl/browser/nacl_browser.h"
 #include "components/nacl/browser/pnacl_translation_cache.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
@@ skipping 12 matching lines @@
 }
 
 void CloseScopedFile(std::unique_ptr<base::File> auto_file_closer) {}
 
 }  // namespace
 
 namespace pnacl {
 
 class FileProxy {
  public:
-  FileProxy(std::unique_ptr<base::File> file,
-            base::WeakPtr<pnacl::PnaclHost> host);
+  FileProxy(std::unique_ptr<base::File> file, PnaclHost* host);
   int Write(scoped_refptr<net::DrainableIOBuffer> buffer);
   void WriteDone(const PnaclHost::TranslationID& id, int result);
 
  private:
   std::unique_ptr<base::File> file_;
-  base::WeakPtr<pnacl::PnaclHost> host_;
+  PnaclHost* host_;
 };
 
-FileProxy::FileProxy(std::unique_ptr<base::File> file,
-                     base::WeakPtr<pnacl::PnaclHost> host)
+FileProxy::FileProxy(std::unique_ptr<base::File> file, PnaclHost* host)
     : file_(std::move(file)), host_(host) {}
 
 int FileProxy::Write(scoped_refptr<net::DrainableIOBuffer> buffer) {
   int rv = file_->Write(0, buffer->data(), buffer->size());
   if (rv == -1)
     PLOG(ERROR) << "FileProxy::Write error";
   return rv;
 }
 
 void FileProxy::WriteDone(const PnaclHost::TranslationID& id, int result) {
-  if (host_) {
-    host_->OnBufferCopiedToTempFile(id, std::move(file_), result);
-  } else {
-    BrowserThread::PostBlockingPoolTask(
-        FROM_HERE,
-        base::Bind(CloseScopedFile, Passed(&file_)));
-  }
+  host_->OnBufferCopiedToTempFile(id, std::move(file_), result);
 }
 
-PnaclHost::PnaclHost()
-    : pending_backend_operations_(0),
-      cache_state_(CacheUninitialized),
-      weak_factory_(this) {}
-
-PnaclHost::~PnaclHost() {
-  // When PnaclHost is destroyed, it's too late to post anything to the cache
-  // thread (it will hang shutdown). So just leak the cache backend.
-  pnacl::PnaclTranslationCache* cache = disk_cache_.release();
-  (void)cache;
-}
+PnaclHost::PnaclHost() = default;
 
 PnaclHost* PnaclHost::GetInstance() {
-  return base::Singleton<PnaclHost>::get();
+  static PnaclHost* instance = nullptr;
+  if (!instance) {
+    instance = new PnaclHost;
+    ANNOTATE_LEAKING_OBJECT_PTR(instance);
+  }
+  return instance;
 }
 
 PnaclHost::PendingTranslation::PendingTranslation()
     : process_handle(base::kNullProcessHandle),
       render_view_id(0),
       nexe_fd(NULL),
       got_nexe_fd(false),
       got_cache_reply(false),
       got_cache_hit(false),
       is_incognito(false),
@@ skipping 51 matching lines @@
 }
 
 void PnaclHost::Init() {
   // Extra check that we're on the real IO thread since this version of
   // Init isn't used in unit tests.
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(thread_checker_.CalledOnValidThread());
   base::FilePath cache_path(GetCachePath());
   if (cache_path.empty() || cache_state_ != CacheUninitialized)
     return;
-  disk_cache_.reset(new pnacl::PnaclTranslationCache());
+  disk_cache_.reset(new PnaclTranslationCache());
   cache_state_ = CacheInitializing;
   int rv = disk_cache_->InitOnDisk(
       cache_path,
-      base::Bind(&PnaclHost::OnCacheInitialized, weak_factory_.GetWeakPtr()));
+      base::Bind(&PnaclHost::OnCacheInitialized, base::Unretained(this)));
   if (rv != net::ERR_IO_PENDING)
     OnCacheInitialized(rv);
 }
 
 // Initialize for testing, optionally using the in-memory backend, and manually
 // setting the temporary file directory instead of using the system directory.
 void PnaclHost::InitForTest(base::FilePath temp_dir, bool in_memory) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  disk_cache_.reset(new pnacl::PnaclTranslationCache());
+  disk_cache_.reset(new PnaclTranslationCache());
   cache_state_ = CacheInitializing;
   temp_dir_ = temp_dir;
   int rv;
   if (in_memory) {
     rv = disk_cache_->InitInMemory(
-      base::Bind(&PnaclHost::OnCacheInitialized, weak_factory_.GetWeakPtr()));
+        base::Bind(&PnaclHost::OnCacheInitialized, base::Unretained(this)));
   } else {
     rv = disk_cache_->InitOnDisk(
-      temp_dir,
-      base::Bind(&PnaclHost::OnCacheInitialized, weak_factory_.GetWeakPtr()));
+        temp_dir,
+        base::Bind(&PnaclHost::OnCacheInitialized, base::Unretained(this)));
   }
   if (rv != net::ERR_IO_PENDING)
     OnCacheInitialized(rv);
 }
 
 ///////////////////////////////////////// Temp files
 
 // Create a temporary file on the blocking pool
 // static
 void PnaclHost::DoCreateTemporaryFile(base::FilePath temp_dir,
@@ skipping 42 matching lines @@
                           const NexeFdCallback& cb) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (cache_state_ == CacheUninitialized) {
     Init();
   }
   if (cache_state_ != CacheReady) {
     // If the backend hasn't yet initialized, try the request again later.
     BrowserThread::PostDelayedTask(BrowserThread::IO,
                                    FROM_HERE,
                                    base::Bind(&PnaclHost::GetNexeFd,
-                                              weak_factory_.GetWeakPtr(),
+                                              base::Unretained(this),
                                               render_process_id,
                                               render_view_id,
                                               pp_instance,
                                               is_incognito,
                                               cache_info,
                                               cb),
                                    base::TimeDelta::FromMilliseconds(
                                        kTranslationCacheInitializationDelayMs));
     return;
   }
@@ skipping 21 matching lines @@
   pt.is_incognito = is_incognito;
   pending_translations_[id] = pt;
   SendCacheQueryAndTempFileRequest(cache_key, id);
 }
 
 // Dispatch the cache read request and the temp file creation request
 // simultaneously; currently we need a temp file regardless of whether the
 // request hits.
 void PnaclHost::SendCacheQueryAndTempFileRequest(const std::string& cache_key,
                                                  const TranslationID& id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   pending_backend_operations_++;
-  disk_cache_->GetNexe(
-      cache_key,
-      base::Bind(
-          &PnaclHost::OnCacheQueryReturn, weak_factory_.GetWeakPtr(), id));
+  disk_cache_->GetNexe(cache_key, base::Bind(&PnaclHost::OnCacheQueryReturn,
+                                             base::Unretained(this), id));
 
   CreateTemporaryFile(
-      base::Bind(&PnaclHost::OnTempFileReturn, weak_factory_.GetWeakPtr(), id));
+      base::Bind(&PnaclHost::OnTempFileReturn, base::Unretained(this), id));
 }
 
 // Callback from the translation cache query. |id| is bound from
 // SendCacheQueryAndTempFileRequest, |net_error| is a net::Error code (which for
 // our purposes means a hit if it's net::OK (i.e. 0). |buffer| is allocated
 // by PnaclTranslationCache and now belongs to PnaclHost.
 // (Bound callbacks must re-lookup the TranslationID because the translation
 // could be cancelled before they get called).
 void PnaclHost::OnCacheQueryReturn(
     const TranslationID& id,
@@ skipping 48 matching lines @@
   PendingTranslation* pt = &entry->second;
   pt->got_nexe_fd = true;
   pt->nexe_fd = new base::File(std::move(file));
   CheckCacheQueryReady(entry);
 }
 
 // Check whether both the cache query and the temp file have returned, and check
 // whether we actually got a hit or not.
 void PnaclHost::CheckCacheQueryReady(
     const PendingTranslationMap::iterator& entry) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   PendingTranslation* pt = &entry->second;
   if (!(pt->got_cache_reply && pt->got_nexe_fd))
     return;
   if (!pt->got_cache_hit) {
     // Check if there is already a pending translation for this file. If there
     // is, we will wait for it to come back, to avoid redundant translations.
     for (PendingTranslationMap::iterator it = pending_translations_.begin();
          it != pending_translations_.end();
          ++it) {
       // Another translation matches if it's a request for the same file,
       if (it->second.cache_key == entry->second.cache_key &&
           // and it's not this translation,
           it->first != entry->first &&
           // and it can be stored in the cache,
           TranslationMayBeCached(it) &&
           // and it's already gotten past this check and returned the miss.
           it->second.got_cache_reply &&
           it->second.got_nexe_fd) {
         return;
       }
     }
     ReturnMiss(entry);
     return;
   }
 
   std::unique_ptr<base::File> file(pt->nexe_fd);
   pt->nexe_fd = NULL;
   pt->got_nexe_fd = false;
-  FileProxy* proxy(new FileProxy(std::move(file), weak_factory_.GetWeakPtr()));
+  FileProxy* proxy(new FileProxy(std::move(file), this));
 
   if (!base::PostTaskAndReplyWithResult(
            BrowserThread::GetBlockingPool(),
            FROM_HERE,
            base::Bind(&FileProxy::Write, base::Unretained(proxy),
                       pt->nexe_read_buffer),
            base::Bind(&FileProxy::WriteDone, base::Owned(proxy),
                       entry->first))) {
     pt->callback.Run(base::File(), false);
   }
@@ skipping 59 matching lines @@
   // translations.
   if (!entry->second.got_nexe_fd || !entry->second.got_cache_reply ||
       !success || !TranslationMayBeCached(entry)) {
     store_nexe = false;
   } else {
     std::unique_ptr<base::File> file(entry->second.nexe_fd);
     entry->second.nexe_fd = NULL;
     entry->second.got_nexe_fd = false;
 
     if (!base::PostTaskAndReplyWithResult(
-             BrowserThread::GetBlockingPool(),
-             FROM_HERE,
-             base::Bind(&PnaclHost::CopyFileToBuffer, Passed(&file)),
-             base::Bind(&PnaclHost::StoreTranslatedNexe,
-                        weak_factory_.GetWeakPtr(),
-                        id))) {
+            BrowserThread::GetBlockingPool(), FROM_HERE,
+            base::Bind(&PnaclHost::CopyFileToBuffer, Passed(&file)),
+            base::Bind(&PnaclHost::StoreTranslatedNexe, base::Unretained(this),
+                       id))) {
       store_nexe = false;
     }
   }
 
   if (!store_nexe) {
     // If store_nexe is true, the fd will be closed by CopyFileToBuffer.
     if (entry->second.got_nexe_fd) {
       std::unique_ptr<base::File> file(entry->second.nexe_fd);
       entry->second.nexe_fd = NULL;
       BrowserThread::PostBlockingPoolTask(
@@ skipping 19 matching lines @@
     LOG(ERROR) << "StoreTranslatedNexe: TranslationID " << id.first << ","
                << id.second << " not found.";
     return;
   }
 
   if (buffer.get() == NULL) {
     LOG(ERROR) << "Error reading translated nexe";
     return;
   }
   pending_backend_operations_++;
-  disk_cache_->StoreNexe(it->second.cache_key,
-                         buffer.get(),
+  disk_cache_->StoreNexe(it->second.cache_key, buffer.get(),
                          base::Bind(&PnaclHost::OnTranslatedNexeStored,
-                                    weak_factory_.GetWeakPtr(),
-                                    it->first));
+                                    base::Unretained(this), it->first));
 }
 
 // After we know the nexe has been stored, we can clean up, and unblock any
 // outstanding requests for the same file.
 // (Bound callbacks must re-lookup the TranslationID because the translation
 // could be cancelled before they get called).
 void PnaclHost::OnTranslatedNexeStored(const TranslationID& id, int net_error) {
   PendingTranslationMap::iterator entry(pending_translations_.find(id));
   pending_backend_operations_--;
   if (entry == pending_translations_.end()) {
     // If the renderer closed while we were storing the nexe, we land here.
     // Make sure we try to de-init.
     DeInitIfSafe();
     return;
   }
   std::string key(entry->second.cache_key);
   pending_translations_.erase(entry);
   RequeryMatchingTranslations(key);
 }
 
 // Check if any pending translations match |key|. If so, re-issue the cache
 // query. In the overlapped miss case, we expect a hit this time, but a miss
 // is also possible in case of an error.
 void PnaclHost::RequeryMatchingTranslations(const std::string& key) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Check for outstanding misses to this same file
   for (PendingTranslationMap::iterator it = pending_translations_.begin();
        it != pending_translations_.end();
        ++it) {
     if (it->second.cache_key == key) {
       // Re-send the cache read request. This time we expect a hit, but if
       // something goes wrong, it will just handle it like a miss.
       it->second.got_cache_reply = false;
       pending_backend_operations_++;
-      disk_cache_->GetNexe(key,
-                           base::Bind(&PnaclHost::OnCacheQueryReturn,
-                                      weak_factory_.GetWeakPtr(),
-                                      it->first));
+      disk_cache_->GetNexe(key, base::Bind(&PnaclHost::OnCacheQueryReturn,
+                                           base::Unretained(this), it->first));
     }
   }
 }
 
 //////////////////// GetNexeFd hit path
 
 void PnaclHost::OnBufferCopiedToTempFile(const TranslationID& id,
                                          std::unique_ptr<base::File> file,
                                          int file_error) {
   DCHECK(thread_checker_.CalledOnValidThread());
   PendingTranslationMap::iterator entry(pending_translations_.find(id));
   if (entry == pending_translations_.end()) {
     BrowserThread::PostBlockingPoolTask(
         FROM_HERE,
         base::Bind(CloseScopedFile, Passed(&file)));
     return;
   }
   if (file_error == -1) {
     // Write error on the temp file. Request a new file and start over.
     BrowserThread::PostBlockingPoolTask(
         FROM_HERE,
         base::Bind(CloseScopedFile, Passed(&file)));
     CreateTemporaryFile(base::Bind(&PnaclHost::OnTempFileReturn,
-                                   weak_factory_.GetWeakPtr(),
-                                   entry->first));
+                                   base::Unretained(this), entry->first));
     return;
   }
   entry->second.callback.Run(*file.get(), true);
   BrowserThread::PostBlockingPoolTask(
       FROM_HERE,
       base::Bind(CloseScopedFile, Passed(&file)));
   pending_translations_.erase(entry);
 }
 
 ///////////////////
@@ skipping 14 matching lines @@
           base::Bind(CloseScopedFile, Passed(&file)));
       std::string key(to_erase->second.cache_key);
       bool may_be_cached = TranslationMayBeCached(to_erase);
       pending_translations_.erase(to_erase);
       // No translations will be waiting for entries that will not be stored.
       if (may_be_cached)
         RequeryMatchingTranslations(key);
     }
   }
   BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&PnaclHost::DeInitIfSafe, weak_factory_.GetWeakPtr()));
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&PnaclHost::DeInitIfSafe, base::Unretained(this)));
 }
 
 ////////////////// Cache data removal
 void PnaclHost::ClearTranslationCacheEntriesBetween(
     base::Time initial_time,
     base::Time end_time,
     const base::Closure& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (cache_state_ == CacheUninitialized) {
     Init();
   }
   if (cache_state_ == CacheInitializing) {
     // If the backend hasn't yet initialized, try the request again later.
     BrowserThread::PostDelayedTask(
-        BrowserThread::IO,
-        FROM_HERE,
+        BrowserThread::IO, FROM_HERE,
         base::Bind(&PnaclHost::ClearTranslationCacheEntriesBetween,
-                   weak_factory_.GetWeakPtr(),
-                   initial_time,
-                   end_time,
-                   callback),
+                   base::Unretained(this), initial_time, end_time, callback),
         base::TimeDelta::FromMilliseconds(
             kTranslationCacheInitializationDelayMs));
     return;
   }
   pending_backend_operations_++;
   int rv = disk_cache_->DoomEntriesBetween(
-      initial_time,
-      end_time,
-      base::Bind(
-          &PnaclHost::OnEntriesDoomed, weak_factory_.GetWeakPtr(), callback));
+      initial_time, end_time, base::Bind(&PnaclHost::OnEntriesDoomed,
+                                         base::Unretained(this), callback));
   if (rv != net::ERR_IO_PENDING)
     OnEntriesDoomed(callback, rv);
 }
 
 void PnaclHost::OnEntriesDoomed(const base::Closure& callback, int net_error) {
   DCHECK(thread_checker_.CalledOnValidThread());
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, callback);
   pending_backend_operations_--;
   // When clearing the cache, the UI is blocked on all the cache-clearing
   // operations, and freeing the backend actually blocks the IO thread. So
   // instead of calling DeInitIfSafe directly, post it for later.
   BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&PnaclHost::DeInitIfSafe, weak_factory_.GetWeakPtr()));
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&PnaclHost::DeInitIfSafe, base::Unretained(this)));
 }
 
 // Destroying the cache backend causes it to post tasks to the cache thread to
-// flush to disk. Because PnaclHost is a singleton, it does not get destroyed
-// until all the browser threads have gone away and it's too late to post
-// anything (attempting to do so hangs shutdown).  So we make sure to destroy it
-// when we no longer have any outstanding operations that need it. These include
-// pending translations, cache clear requests, and requests to read or write
-// translated nexes.  We check when renderers close, when cache clear requests
-// finish, and when backend operations complete.
+// flush to disk. PnaclHost is leaked on shutdown because registering it as a
+// Singleton with AtExitManager would result in it not being destroyed until all
+// the browser threads have gone away and it's too late to post anything
+// (attempting to do so hangs shutdown) at that point anyways.  So we make sure
+// to destroy it when we no longer have any outstanding operations that need it.
+// These include pending translations, cache clear requests, and requests to
+// read or write translated nexes.  We check when renderers close, when cache
+// clear requests finish, and when backend operations complete.
 
 // It is not safe to delete the backend while it is initializing, nor if it has
 // outstanding entry open requests; it is in theory safe to delete it with
 // outstanding read/write requests, but because that distinction is hidden
 // inside PnaclTranslationCache, we do not delete the backend if there are any
 // backend requests in flight.  As a last resort in the destructor, we just leak
 // the backend to avoid hanging shutdown.
 void PnaclHost::DeInitIfSafe() {
   DCHECK(pending_backend_operations_ >= 0);
   if (pending_translations_.empty() &&
       pending_backend_operations_ <= 0 &&
       cache_state_ == CacheReady) {
     cache_state_ = CacheUninitialized;
     disk_cache_.reset();
   }
 }
 
 }  // namespace pnacl