Omnibox - ClipboardRecentContent - Make Proper Singleton

components/open_from_clipboard/clipboard_recent_content.cc

 // Copyright 2015 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/open_from_clipboard/clipboard_recent_content.h"
 
 #include "url/url_constants.h"
 
 namespace {
-ClipboardRecentContent* g_clipboard_recent_content = nullptr;
-
 // Schemes appropriate for suggestion by ClipboardRecentContent.
 const char* kAuthorizedSchemes[] = {
     url::kAboutScheme, url::kDataScheme, url::kHttpScheme, url::kHttpsScheme,
     // TODO(mpearson): add support for chrome:// URLs.  Right now the scheme
     // for that lives in content and is accessible via
     // GetEmbedderRepresentationOfAboutScheme() or content::kChromeUIScheme
     // TODO(mpearson): when adding desktop support, add kFileScheme, kFtpScheme,
     // and kGopherScheme.
 };
 
 }  // namespace
 
 ClipboardRecentContent::ClipboardRecentContent() {}
 
 ClipboardRecentContent::~ClipboardRecentContent() {
-  g_clipboard_recent_content = nullptr;
 }
 
 // static
 ClipboardRecentContent* ClipboardRecentContent::GetInstance() {
-  return g_clipboard_recent_content;
+  return UniquePtrToInstance()->get();
 }
 
 // static
-void ClipboardRecentContent::SetInstance(ClipboardRecentContent* instance) {
-  g_clipboard_recent_content = instance;
+void ClipboardRecentContent::SetInstance(
+    std::unique_ptr<ClipboardRecentContent> new_instance) {
+  (*UniquePtrToInstance()) = std::move(new_instance);
 }
 
 // static
 bool ClipboardRecentContent::IsAppropriateSuggestion(const GURL& url) {
   // Check to make sure it's a scheme we're willing to suggest.
   for (const auto* authorized_scheme : kAuthorizedSchemes) {
     if (url.SchemeIs(authorized_scheme))
       return true;
   }
 
   // Not a scheme we're allowed to return.
   return false;
 }
+
+// static
+std::unique_ptr<ClipboardRecentContent>*
+ClipboardRecentContent::UniquePtrToInstance() {

[Peter Kasting, 2017/04/10 21:59:22]

Why do this as a static member function instead of a file-scoped unique_ptr? 
The latter seems more direct.

[Mark P, 2017/04/10 22:05:22]

jif@ wrote:
"I don't think we are allowed to have global static unique_ptr,"

Do you disagree?

[Peter Kasting, 2017/04/10 22:18:34]

On thinking about it more I think both methods are problematic.  The style guide
basically bans globals/statics that run destructors.  So no matter where we
declare it this is a problem.  We want the object to not be destroyed on exit.

We could do this with CR_DEFINE_STATIC_LOCAL here instead.  But this all feels
like overkill.  Why not leave the interface above as taking a scoped_ptr (to
indicate ownership transfer), but under the hood store this in a raw pointer
like before?

ClipboardRecentContent* g_clipboard_recent_content = nullptr;

void ClipboardRecentContent::SetInstance(
    std::unique_ptr<ClipboardRecentContent> new_instance) {
  delete g_clipboard_recent_content;
  g_clipboard_recent_content = new_instance.release();
}

[Mark P, 2017/04/10 23:10:14]

Okay.

This is also fine with me.  Done.

+  static std::unique_ptr<ClipboardRecentContent> instance;
+  return &instance;
+}