Tweaks to PPB_VarArrayBuffer in preperation for taking out of Dev.

ppapi/cpp/dev/var_array_buffer_dev.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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 "ppapi/cpp/dev/var_array_buffer_dev.h"
 
+#include <limits>
+
 #include "ppapi/c/dev/ppb_var_array_buffer_dev.h"
 #include "ppapi/cpp/logging.h"
 #include "ppapi/cpp/module_impl.h"
 
 namespace pp {
 
 namespace {
 
 template <> const char* interface_name<PPB_VarArrayBuffer_Dev>() {
   return PPB_VAR_ARRAY_BUFFER_DEV_INTERFACE;
 }
 
 }  // namespace
 
-VarArrayBuffer_Dev::VarArrayBuffer_Dev(const Var& var)
-    : Var(var), buffer_(NULL) {
-  if (var.is_array_buffer()) {
-    buffer_ = Map();
+VarArrayBuffer_Dev::VarArrayBuffer_Dev(const Var& var) : Var(var) {
+  if (!var.is_array_buffer()) {
+    PP_NOTREACHED();
+    var_ = PP_MakeNull();
+  }
+}
+
+VarArrayBuffer_Dev::VarArrayBuffer_Dev(uint32_t size_in_bytes) {
+  if (has_interface<PPB_VarArrayBuffer_Dev>()) {
+    var_ = get_interface<PPB_VarArrayBuffer_Dev>()->Create(size_in_bytes);
+    needs_release_ = true;
   } else {
     PP_NOTREACHED();
     var_ = PP_MakeNull();
   }
 }
 
-VarArrayBuffer_Dev::VarArrayBuffer_Dev(uint32_t size_in_bytes)
-    : buffer_(NULL) {
-  if (has_interface<PPB_VarArrayBuffer_Dev>()) {
-    var_ = get_interface<PPB_VarArrayBuffer_Dev>()->Create(size_in_bytes);
-    needs_release_ = true;
-    buffer_ = Map();
-  } else {
-    PP_NOTREACHED();
-    var_ = PP_MakeNull();
-  }
+pp::VarArrayBuffer_Dev& VarArrayBuffer_Dev::operator=(
+    const VarArrayBuffer_Dev& other) {
+  Var::operator=(other);

[bbudge, 2012/01/26 17:20:26]

Isn't it a problem that you can assign a non-ArrayBuffer var to an ArrayBuffer
(using the base operator=)? Seems like operator= needs to be virtual on the base
so you can check that it's the right type (VarArrayBuffer) in your override.

[dmichael (off chromium), 2012/01/26 18:33:48]

Good point; I was only really thinking about static types. Done.

+  return *this;
 }
 
 uint32_t VarArrayBuffer_Dev::ByteLength() const {
-  if (has_interface<PPB_VarArrayBuffer_Dev>()) {
-    return get_interface<PPB_VarArrayBuffer_Dev>()->ByteLength(var_);
-  }
-  PP_NOTREACHED();
-  return 0;
-}
-
-const void* VarArrayBuffer_Dev::Map() const {
-  return const_cast<VarArrayBuffer_Dev*>(this)->Map();
+  uint32_t byte_length = std::numeric_limits<uint32_t>::max();
+  if (is_array_buffer() && has_interface<PPB_VarArrayBuffer_Dev>())
+    get_interface<PPB_VarArrayBuffer_Dev>()->ByteLength(var_, &byte_length);
+  else
+    PP_NOTREACHED();
+  return byte_length;
 }
 
 void* VarArrayBuffer_Dev::Map() {
-  // TODO(dmichael): This is not thread-safe.
-  if (buffer_)
-    return buffer_;
-  if (has_interface<PPB_VarArrayBuffer_Dev>()) {
-    buffer_ = get_interface<PPB_VarArrayBuffer_Dev>()->Map(var_);
-    return buffer_;
-  }
+  if (is_array_buffer() && has_interface<PPB_VarArrayBuffer_Dev>())
+    return get_interface<PPB_VarArrayBuffer_Dev>()->Map(var_);
   PP_NOTREACHED();
-  return 0;
+  return NULL;
+}
+
+void VarArrayBuffer_Dev::Unmap() {
+  if (is_array_buffer() && has_interface<PPB_VarArrayBuffer_Dev>())
+    get_interface<PPB_VarArrayBuffer_Dev>()->Map(var_);

[bbudge, 2012/01/26 17:20:26]

Reading the code, it still seems weird to see Unmap call Map. Maybe a comment
explaining why?

[dmichael (off chromium), 2012/01/26 18:33:48]

Gah. You were completely right the first time. I read it as if you were
commenting on the example code in the header for some reason, rather than
looking at the actual context of your comment. Sorry, and thanks for keeping me
honest. Fixed.

+  else
+    PP_NOTREACHED();
 }
 
 }  // namespace pp