bindings: Use Maybe version of Get in bindings/

bindings: Use Maybe version of Get in bindings/

Except for:
- bindings/core/v8/V8Binding
- bindings/core/v8/ScriptDebugServer
- generated code
- custom bindings

BUG=462402
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193269

[bashi, 2015-04-07 07:15:37]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org, yukishiino@chromium.org

[bashi, 2015-04-07 07:15:37]

PTAL?

[Yuki, 2015-04-07 07:37:57]

lgtm

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/NPV...
File Source/bindings/core/v8/NPV8Object.cpp (right):

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/NPV...
Source/bindings/core/v8/NPV8Object.cpp:537: v8::Isolate* isolate =
v8::Isolate::GetCurrent();
Can you remove this line?

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/NPV...
Source/bindings/core/v8/NPV8Object.cpp:593: v8::Isolate* isolate =
v8::Isolate::GetCurrent();
Can you remove this line?

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
File Source/bindings/core/v8/ScriptValueSerializer.cpp (right):

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
Source/bindings/core/v8/ScriptValueSerializer.cpp:501: return
serializer.handleError(JSException, "", this);
Can we add any descriptions for the error?

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
Source/bindings/core/v8/ScriptValueSerializer.cpp:523: return
serializer.handleError(JSException, "", this);
Can we add any descriptions for the error?

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
Source/bindings/core/v8/ScriptValueSerializer.cpp:556: return
serializer.handleError(JSException, "", this);
Can we add any descriptions for the error?

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/V8P...
File Source/bindings/core/v8/V8PerIsolateData.cpp (right):

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/V8P...
Source/bindings/core/v8/V8PerIsolateData.cpp:245: if
(!info.Callee()->Get(isolate->GetCurrentContext(), v8AtomicString(isolate,
"toString")).ToLocal(&value) ||  !value->IsFunction()) {
nit: two spaces after ||.

[bashi, 2015-04-07 08:12:13]

Patchset #2 (id:20001) has been deleted

[bashi, 2015-04-07 08:46:02]

Patchset #2 (id:40001) has been deleted

[bashi, 2015-04-07 08:47:01]

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/NPV...
File Source/bindings/core/v8/NPV8Object.cpp (right):

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/NPV...
Source/bindings/core/v8/NPV8Object.cpp:537: v8::Isolate* isolate =
v8::Isolate::GetCurrent();
On 2015/04/07 07:37:57, Yuki wrote:
> Can you remove this line?

Done.

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/NPV...
Source/bindings/core/v8/NPV8Object.cpp:593: v8::Isolate* isolate =
v8::Isolate::GetCurrent();
On 2015/04/07 07:37:57, Yuki wrote:
> Can you remove this line?

Done.

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
File Source/bindings/core/v8/ScriptValueSerializer.cpp (right):

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
Source/bindings/core/v8/ScriptValueSerializer.cpp:501: return
serializer.handleError(JSException, "", this);
On 2015/04/07 07:37:57, Yuki wrote:
> Can we add any descriptions for the error?

Done.

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
Source/bindings/core/v8/ScriptValueSerializer.cpp:523: return
serializer.handleError(JSException, "", this);
On 2015/04/07 07:37:57, Yuki wrote:
> Can we add any descriptions for the error?

Done.

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/Scr...
Source/bindings/core/v8/ScriptValueSerializer.cpp:556: return
serializer.handleError(JSException, "", this);
On 2015/04/07 07:37:57, Yuki wrote:
> Can we add any descriptions for the error?

Done.

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/V8P...
File Source/bindings/core/v8/V8PerIsolateData.cpp (right):

https://codereview.chromium.org/1067763002/diff/1/Source/bindings/core/v8/V8P...
Source/bindings/core/v8/V8PerIsolateData.cpp:245: if
(!info.Callee()->Get(isolate->GetCurrentContext(), v8AtomicString(isolate,
"toString")).ToLocal(&value) ||  !value->IsFunction()) {
On 2015/04/07 07:37:57, Yuki wrote:
> nit: two spaces after ||.

Done.

[haraken, 2015-04-07 08:56:43]

LGTM

https://codereview.chromium.org/1067763002/diff/60001/Source/bindings/core/v8...
File Source/bindings/core/v8/V8PerContextData.cpp (right):

https://codereview.chromium.org/1067763002/diff/60001/Source/bindings/core/v8...
Source/bindings/core/v8/V8PerContextData.cpp:57:
ASSERT(objectValue->IsObject());

This ASSERT won't be needed, since the following As has the check.

[bashi, 2015-04-07 09:37:31]

https://codereview.chromium.org/1067763002/diff/60001/Source/bindings/core/v8...
File Source/bindings/core/v8/V8PerContextData.cpp (right):

https://codereview.chromium.org/1067763002/diff/60001/Source/bindings/core/v8...
Source/bindings/core/v8/V8PerContextData.cpp:57:
ASSERT(objectValue->IsObject());
On 2015/04/07 08:56:43, haraken wrote:
> 
> This ASSERT won't be needed, since the following As has the check.

Removed.

https://codereview.chromium.org/1067763002/diff/60001/Source/bindings/core/v8...
Source/bindings/core/v8/V8PerContextData.cpp:114: if
(function->Get(currentContext, v8AtomicString(m_isolate,
"prototype")).ToLocal(&prototypeValue) && prototypeValue->IsObject()) {
Chatted offline and we think it would be better to crash than returning a
half-baked function object.

https://codereview.chromium.org/1067763002/diff/60001/Source/bindings/core/v8...
Source/bindings/core/v8/V8PerContextData.cpp:122: return
v8::Local<v8::Function>();
This would also be RELEASE_ASSERT_NOT_REACHED(), but I'd like to do that in a
follow-up CL.

[bashi, 2015-04-07 12:51:37]

The CQ bit was checked by bashi@chromium.org

[bashi, 2015-04-07 12:51:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
yukishiino@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1067763002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2015-04-07 12:51:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1067763002/80001

[commit-bot: I haz the power, 2015-04-07 14:05:36]

Committed patchset #3 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193269