bindings: Add empty checks for toV8()

Source/web/WebPluginContainerImpl.cpp

Index: Source/web/WebPluginContainerImpl.cpp
diff --git a/Source/web/WebPluginContainerImpl.cpp b/Source/web/WebPluginContainerImpl.cpp
index 55e2802ecbbcb085f8b1adc129d7654ae5a40937..f310e0ec752bc4915331a0e3940a1cea5abd068d 100644
--- a/Source/web/WebPluginContainerImpl.cpp
+++ b/Source/web/WebPluginContainerImpl.cpp
@@ -463,7 +463,7 @@ v8::Local<v8::Object> WebPluginContainerImpl::v8ObjectForElement()
         return v8::Local<v8::Object>();
 
     v8::Handle<v8::Value> v8value = toV8(m_element.get(), scriptState->context()->Global(), scriptState->isolate());
-    ASSERT(v8value->IsObject());
+    ASSERT(!v8value.IsEmpty() && v8value->IsObject());

[haraken, 2015/04/28 04:24:59]

Who guarantees it is not empty?

Also we can remove the IsObject check.

[bashi, 2015/04/28 05:41:53]

The existing ASSERT() implies this should not be empty, maybe?

[haraken, 2015/04/28 05:50:34]

I think that is exactly what we're fixing with introducing the Maybe version. We
need to assume that any V8 API that returns a Maybe can return an empty handle.

[bashi, 2015/04/28 06:41:14]

Yeah, let's use !IsEmpty() assertion to see this assumption is correct. I'll do
immediate revert if this is a problem.

[Yuki, 2015/04/28 06:43:46]

The problem is, ASSERT will be noop in release build, I think.
How can we detect the problem in the real world?

[haraken, 2015/04/28 06:46:08]

If you argue that we can go with ASSERT here, that means we can go with ASSERTs
in many other cases. (And the ASSERTs will rarely hit.)

IMHO, we should avoid using ASSERTs (or ToLocalChecked) unless we're pretty sure
that the API should not return an empty handle. Also we should limit the places.

[bashi, 2015/04/28 06:55:49]

Ok, changed to return empty handle.

That's said, I'd prefer using ASSERTs or immediate crashes if possible because
returning empty handles spreads out the need for empty checks. That's what I'm
suffering from now :(

 
     return v8::Handle<v8::Object>::Cast(v8value);
 }