Rewrite most of the scopers in //base/mac to use ScopedTypeRef or Sco…

…pedGeneric. This removes a lot of duplicated code in favor of using type aliases and minimal Traits structs. R=mark@chromium.org Review URL: https://codereview.chromium.org/1551943002 . Cr-Commit-Position: refs/heads/master@{#367322}
Denger-Network · Jan 4, 2016 · cfd6ed5 · cfd6ed5
1 parent 5a60672
commit cfd6ed5
Show file tree

Hide file tree

Showing 21 changed files with 168 additions and 446 deletions.
diff --git a/base/mac/launchd.cc b/base/mac/launchd.cc
@@ -18,7 +18,7 @@ launch_data_t MessageForJob(const std::string& job_label,
                             const char* operation) {
   // launch_data_alloc returns something that needs to be freed.
   ScopedLaunchData message(launch_data_alloc(LAUNCH_DATA_DICTIONARY));
-  if (!message) {
+  if (!message.is_valid()) {
     LOG(ERROR) << "launch_data_alloc";
     return NULL;
   }
@@ -28,38 +28,38 @@ launch_data_t MessageForJob(const std::string& job_label,
   // called, so put it in a scoper and .release() it when given to the
   // dictionary.
   ScopedLaunchData job_label_launchd(launch_data_new_string(job_label.c_str()));
-  if (!job_label_launchd) {
+  if (!job_label_launchd.is_valid()) {
     LOG(ERROR) << "launch_data_new_string";
     return NULL;
   }
 
-  if (!launch_data_dict_insert(message,
-                               job_label_launchd.release(),
+  if (!launch_data_dict_insert(message.get(), job_label_launchd.release(),
                                operation)) {
     return NULL;
   }
 
-  return launch_msg(message);
+  return launch_msg(message.get());
 }
 
 pid_t PIDForJob(const std::string& job_label) {
   ScopedLaunchData response(MessageForJob(job_label, LAUNCH_KEY_GETJOB));
-  if (!response) {
+  if (!response.is_valid()) {
     return -1;
   }
 
-  launch_data_type_t response_type = launch_data_get_type(response);
+  launch_data_type_t response_type = launch_data_get_type(response.get());
   if (response_type != LAUNCH_DATA_DICTIONARY) {
     if (response_type == LAUNCH_DATA_ERRNO) {
-      LOG(ERROR) << "PIDForJob: error " << launch_data_get_errno(response);
+      LOG(ERROR) << "PIDForJob: error "
+                 << launch_data_get_errno(response.get());
     } else {
       LOG(ERROR) << "PIDForJob: expected dictionary, got " << response_type;
     }
     return -1;
   }
 
-  launch_data_t pid_data = launch_data_dict_lookup(response,
-                                                   LAUNCH_JOBKEY_PID);
+  launch_data_t pid_data =
+      launch_data_dict_lookup(response.get(), LAUNCH_JOBKEY_PID);
   if (!pid_data)
     return 0;
 

diff --git a/base/mac/scoped_block.h b/base/mac/scoped_block.h
@@ -7,83 +7,27 @@
 
 #include <Block.h>
 
-#include "base/compiler_specific.h"
-#include "base/memory/scoped_policy.h"
+#include "base/mac/scoped_typeref.h"
 
 namespace base {
 namespace mac {
 
-// ScopedBlock<> is patterned after ScopedCFTypeRef<>, but uses Block_copy() and
-// Block_release() instead of CFRetain() and CFRelease().
-
-template<typename B>
-class ScopedBlock {
- public:
-  explicit ScopedBlock(
-      B block = nullptr,
-      base::scoped_policy::OwnershipPolicy policy = base::scoped_policy::ASSUME)
-      : block_(block) {
-    if (block_ && policy == base::scoped_policy::RETAIN)
-      block_ = Block_copy(block);
-  }
-
-  ScopedBlock(const ScopedBlock<B>& that)
-      : block_(that.block_) {
-    if (block_)
-      block_ = Block_copy(block_);
-  }
-
-  ~ScopedBlock() {
-    if (block_)
-      Block_release(block_);
-  }
-
-  ScopedBlock& operator=(const ScopedBlock<B>& that) {
-    reset(that.get(), base::scoped_policy::RETAIN);
-    return *this;
-  }
-
-  void reset(B block = nullptr,
-             base::scoped_policy::OwnershipPolicy policy =
-                 base::scoped_policy::ASSUME) {
-    if (block && policy == base::scoped_policy::RETAIN)
-      block = Block_copy(block);
-    if (block_)
-      Block_release(block_);
-    block_ = block;
-  }
+namespace internal {
 
-  bool operator==(B that) const {
-    return block_ == that;
-  }
-
-  bool operator!=(B that) const {
-    return block_ != that;
-  }
-
-  operator B() const {
-    return block_;
-  }
-
-  B get() const {
-    return block_;
-  }
+template <typename B>
+struct ScopedBlockTraits {
+  static B InvalidValue() { return nullptr; }
+  static B Retain(B block) { return Block_copy(block); }
+  static void Release(B block) { Block_release(block); }
+};
 
-  void swap(ScopedBlock& that) {
-    B temp = that.block_;
-    that.block_ = block_;
-    block_ = temp;
-  }
+}  // namespace internal
 
-  B release() WARN_UNUSED_RESULT {
-    B temp = block_;
-    block_ = nullptr;
-    return temp;
-  }
+// ScopedBlock<> is patterned after ScopedCFTypeRef<>, but uses Block_copy() and
+// Block_release() instead of CFRetain() and CFRelease().
 
- private:
-  B block_;
-};
+template <typename B>
+using ScopedBlock = ScopedTypeRef<B, internal::ScopedBlockTraits<B>>;
 
 }  // namespace mac
 }  // namespace base

diff --git a/base/mac/scoped_cffiledescriptorref.h b/base/mac/scoped_cffiledescriptorref.h
@@ -7,67 +7,31 @@
 
 #include <CoreFoundation/CoreFoundation.h>
 
-#include "base/compiler_specific.h"
-#include "base/macros.h"
+#include "base/scoped_generic.h"
 
 namespace base {
 namespace mac {
 
+namespace internal {
+
+struct ScopedCFFileDescriptorRefTraits {
+  static CFFileDescriptorRef InvalidValue() { return nullptr; }
+  static void Free(CFFileDescriptorRef ref) {
+    CFFileDescriptorInvalidate(ref);
+    CFRelease(ref);
+  }
+};
+
+}  // namespace internal
+
 // ScopedCFFileDescriptorRef is designed after ScopedCFTypeRef<>. On
 // destruction, it will invalidate the file descriptor.
 // ScopedCFFileDescriptorRef (unlike ScopedCFTypeRef<>) does not support RETAIN
 // semantics, copying, or assignment, as doing so would increase the chances
 // that a file descriptor is invalidated while still in use.
-class ScopedCFFileDescriptorRef {
- public:
-  explicit ScopedCFFileDescriptorRef(CFFileDescriptorRef fdref = NULL)
-      : fdref_(fdref) {
-  }
-
-  ~ScopedCFFileDescriptorRef() {
-    if (fdref_) {
-      CFFileDescriptorInvalidate(fdref_);
-      CFRelease(fdref_);
-    }
-  }
-
-  void reset(CFFileDescriptorRef fdref = NULL) {
-    if (fdref_ == fdref)
-      return;
-    if (fdref_) {
-      CFFileDescriptorInvalidate(fdref_);
-      CFRelease(fdref_);
-    }
-    fdref_ = fdref;
-  }
-
-  bool operator==(CFFileDescriptorRef that) const {
-    return fdref_ == that;
-  }
-
-  bool operator!=(CFFileDescriptorRef that) const {
-    return fdref_ != that;
-  }
-
-  operator CFFileDescriptorRef() const {
-    return fdref_;
-  }
-
-  CFFileDescriptorRef get() const {
-    return fdref_;
-  }
-
-  CFFileDescriptorRef release() WARN_UNUSED_RESULT {
-    CFFileDescriptorRef temp = fdref_;
-    fdref_ = NULL;
-    return temp;
-  }
-
- private:
-  CFFileDescriptorRef fdref_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedCFFileDescriptorRef);
-};
+using ScopedCFFileDescriptorRef =
+    ScopedGeneric<CFFileDescriptorRef,
+                  internal::ScopedCFFileDescriptorRefTraits>;
 
 }  // namespace mac
 }  // namespace base

diff --git a/base/mac/scoped_cftyperef.h b/base/mac/scoped_cftyperef.h
@@ -30,8 +30,9 @@ namespace internal {
 template<typename CFT>
 struct ScopedCFTypeRefTraits {
   static CFT InvalidValue() { return nullptr; }
-  static void Retain(CFT object) {
+  static CFT Retain(CFT object) {
     CFRetain(object);
+    return object;
   }
   static void Release(CFT object) {
     CFRelease(object);

diff --git a/base/mac/scoped_ioobject.h b/base/mac/scoped_ioobject.h
@@ -7,66 +7,28 @@
 
 #include <IOKit/IOKitLib.h>
 
-#include "base/compiler_specific.h"
-#include "base/macros.h"
+#include "base/mac/scoped_typeref.h"
 
 namespace base {
 namespace mac {
 
-// Just like ScopedCFTypeRef but for io_object_t and subclasses.
-template<typename IOT>
-class ScopedIOObject {
- public:
-  typedef IOT element_type;
-
-  explicit ScopedIOObject(IOT object = IO_OBJECT_NULL)
-      : object_(object) {
-  }
-
-  ~ScopedIOObject() {
-    if (object_)
-      IOObjectRelease(object_);
-  }
-
-  void reset(IOT object = IO_OBJECT_NULL) {
-    if (object_)
-      IOObjectRelease(object_);
-    object_ = object;
-  }
-
-  bool operator==(IOT that) const {
-    return object_ == that;
-  }
-
-  bool operator!=(IOT that) const {
-    return object_ != that;
-  }
-
-  operator IOT() const {
-    return object_;
-  }
-
-  IOT get() const {
-    return object_;
-  }
+namespace internal {
 
-  void swap(ScopedIOObject& that) {
-    IOT temp = that.object_;
-    that.object_ = object_;
-    object_ = temp;
-  }
-
-  IOT release() WARN_UNUSED_RESULT {
-    IOT temp = object_;
-    object_ = IO_OBJECT_NULL;
-    return temp;
+template <typename IOT>
+struct ScopedIOObjectTraits {
+  static IOT InvalidValue() { return IO_OBJECT_NULL; }
+  static IOT Retain(IOT iot) {
+    IOObjectRetain(iot);
+    return iot;
   }
+  static void Release(IOT iot) { IOObjectRelease(iot); }
+};
 
- private:
-  IOT object_;
+}  // namespce internal
 
-  DISALLOW_COPY_AND_ASSIGN(ScopedIOObject);
-};
+// Just like ScopedCFTypeRef but for io_object_t and subclasses.
+template <typename IOT>
+using ScopedIOObject = ScopedTypeRef<IOT, internal::ScopedIOObjectTraits<IOT>>;
 
 }  // namespace mac
 }  // namespace base