Remove unsafe access hacks from ScopedPersistent.

BUG=236290

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222102

Review URL: https://chromiumcodereview.appspot.com/23636015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222225 0039d316-1c4b-4281-b951-d872f2087c98

chrome/renderer/extensions/activity_log_converter_strategy_unittest.cc

@@ -19,7 +19,7 @@ class ActivityLogConverterStrategyTest : public testing::Test {
       : isolate_(v8::Isolate::GetCurrent())
       , handle_scope_(isolate_)
       , context_(v8::Context::New(isolate_))
-      , context_scope_(context_.get()) {
+      , context_scope_(context()) {
   }
 
  protected:
@@ -32,7 +32,7 @@ class ActivityLogConverterStrategyTest : public testing::Test {
 
   testing::AssertionResult VerifyNull(v8::Local<v8::Value> v8_value) {
     scoped_ptr<base::Value> value(
-        converter_->FromV8Value(v8_value, context_.get()));
+        converter_->FromV8Value(v8_value, context()));
     if (value->IsType(base::Value::TYPE_NULL))
       return testing::AssertionSuccess();
     return testing::AssertionFailure();
@@ -42,7 +42,7 @@ class ActivityLogConverterStrategyTest : public testing::Test {
                                          bool expected) {
     bool out;
     scoped_ptr<base::Value> value(
-        converter_->FromV8Value(v8_value, context_.get()));
+        converter_->FromV8Value(v8_value, context()));
     if (value->IsType(base::Value::TYPE_BOOLEAN)
         && value->GetAsBoolean(&out)
         && out == expected)
@@ -54,7 +54,7 @@ class ActivityLogConverterStrategyTest : public testing::Test {
                                          int expected) {
     int out;
     scoped_ptr<base::Value> value(
-        converter_->FromV8Value(v8_value, context_.get()));
+        converter_->FromV8Value(v8_value, context()));
     if (value->IsType(base::Value::TYPE_INTEGER)
         && value->GetAsInteger(&out)
         && out == expected)
@@ -66,7 +66,7 @@ class ActivityLogConverterStrategyTest : public testing::Test {
                                         double expected) {
     double out;
     scoped_ptr<base::Value> value(
-        converter_->FromV8Value(v8_value, context_.get()));
+        converter_->FromV8Value(v8_value, context()));
     if (value->IsType(base::Value::TYPE_DOUBLE)
         && value->GetAsDouble(&out)
         && out == expected)
@@ -78,7 +78,7 @@ class ActivityLogConverterStrategyTest : public testing::Test {
                                         const std::string& expected) {
     std::string out;
     scoped_ptr<base::Value> value(
-        converter_->FromV8Value(v8_value, context_.get()));
+        converter_->FromV8Value(v8_value, context()));
     if (value->IsType(base::Value::TYPE_STRING)
         && value->GetAsString(&out)
         && out == expected)
@@ -86,6 +86,10 @@ class ActivityLogConverterStrategyTest : public testing::Test {
     return testing::AssertionFailure();
   }
 
+  v8::Handle<v8::Context> context() const {
+    return context_.NewHandle(isolate_);
+  }
+
   v8::Isolate* isolate_;
   v8::HandleScope handle_scope_;
   ScopedPersistent<v8::Context> context_;
@@ -155,4 +159,3 @@ TEST_F(ActivityLogConverterStrategyTest, ConversionTest) {
 }
 
 }  // namespace extensions
-

chrome/renderer/extensions/chrome_v8_context.cc

@@ -143,7 +143,7 @@ void ChromeV8Context::OnResponseReceived(const std::string& name,
     v8::Integer::New(request_id),
     v8::String::New(name.c_str()),
     v8::Boolean::New(success),
-    converter->ToV8Value(&response, v8_context_.get()),
+    converter->ToV8Value(&response, v8_context_.NewHandle(isolate())),
     v8::String::New(error.c_str())
   };
 

chrome/renderer/extensions/chrome_v8_context.h

@@ -43,11 +43,11 @@ class ChromeV8Context : public RequestSender::Source {
   // Returns true if this context is still valid, false if it isn't.
   // A context becomes invalid via Invalidate().
   bool is_valid() const {
-    return !v8_context_.get().IsEmpty();
+    return !v8_context_.IsEmpty();
   }
 
   v8::Handle<v8::Context> v8_context() const {
-    return v8_context_.get();
+    return v8_context_.NewHandle(v8::Isolate::GetCurrent());
   }
 
   const Extension* extension() const {

chrome/renderer/extensions/messaging_bindings.cc

@@ -192,12 +192,13 @@ class ExtensionImpl : public extensions::ChromeV8Extension {
 
     void RunCallback() {
       v8::HandleScope handle_scope(isolate_);
-      v8::Handle<v8::Context> context = callback_->CreationContext();
+      v8::Handle<v8::Function> callback = callback_.NewHandle(isolate_);
+      v8::Handle<v8::Context> context = callback->CreationContext();
       if (context.IsEmpty())
         return;
       v8::Context::Scope context_scope(context);
       WebKit::WebScopedMicrotaskSuppression suppression;
-      callback_->Call(context->Global(), 0, NULL);
+      callback->Call(context->Global(), 0, NULL);
     }
 
     extensions::ScopedPersistent<v8::Object> object_;

chrome/renderer/extensions/object_backed_native_handler.cc

@@ -32,7 +32,7 @@ ObjectBackedNativeHandler::~ObjectBackedNativeHandler() {
 }
 
 v8::Handle<v8::Object> ObjectBackedNativeHandler::NewInstance() {
-  return object_template_->NewInstance();
+  return object_template_.NewHandle(v8::Isolate::GetCurrent())->NewInstance();
 }
 
 // static
@@ -68,7 +68,7 @@ void ObjectBackedNativeHandler::RouteFunction(
                   v8::External::New(new HandlerFunction(handler_function)));
   v8::Handle<v8::FunctionTemplate> function_template =
       v8::FunctionTemplate::New(Router, local_data);
-  object_template_->Set(name.c_str(), function_template);
+  object_template_.NewHandle(isolate)->Set(name.c_str(), function_template);
   router_data_.push_back(UnsafePersistent<v8::Object>(&data));
 }
 

chrome/renderer/extensions/render_view_observer_natives.cc

@@ -40,7 +40,7 @@ class LoadWatcher : public content::RenderViewObserver {
   void CallbackAndDie(bool succeeded) {
     v8::HandleScope handle_scope(context_->isolate());
     v8::Handle<v8::Value> args[] = { v8::Boolean::New(succeeded) };
-    context_->CallFunction(callback_.get(), 1, args);
+    context_->CallFunction(callback_.NewHandle(context_->isolate()), 1, args);
     delete this;
   }
 

chrome/renderer/extensions/scoped_persistent.h

@@ -40,15 +40,8 @@ class ScopedPersistent {
     handle_.Clear();
   }
 
-  v8::Handle<T> operator->() const {
-    return get();
-  }
-
-  // TODO(dcarney): Remove this function
-  // This is an unsafe access to the underlying handle
-  v8::Handle<T> get() const {
-    return *reinterpret_cast<v8::Handle<T>*>(
-      const_cast<v8::Persistent<T>* >(&handle_));
+  bool IsEmpty() const {
+    return handle_.IsEmpty();
   }
 
   v8::Handle<T> NewHandle() const {
@@ -57,6 +50,12 @@ class ScopedPersistent {
     return v8::Local<T>::New(GetIsolate(handle_), handle_);
   }
 
+  v8::Handle<T> NewHandle(v8::Isolate* isolate) const {
+    if (handle_.IsEmpty())
+      return v8::Local<T>();
+    return v8::Local<T>::New(isolate, handle_);
+  }
+
   template<typename P>
   void MakeWeak(P* parameters,
                 typename v8::WeakReferenceCallbacks<T, P>::Revivable callback) {

chrome/renderer/extensions/v8_schema_registry.cc

@@ -108,12 +108,12 @@ v8::Handle<v8::Context> V8SchemaRegistry::GetOrCreateContext(
     v8::Isolate* isolate) {
   // It's ok to create local handles in this function, since this is only called
   // when we have a HandleScope.
-  if (context_.get().IsEmpty()) {
+  if (context_.IsEmpty()) {
     v8::Handle<v8::Context> context = v8::Context::New(isolate);
     context_.reset(context);
     return context;
   }
-  return v8::Local<v8::Context>::New(isolate, context_.get());
+  return context_.NewHandle(isolate);
 }
 
 }  // namespace extensions