Index: third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp |
diff --git a/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp b/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp |
index 5f7ee2bde1564c580bd5d0eefd9fd349b2491cc5..9d459314a02907a86d823cba7f2e2d698089751e 100644 |
--- a/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp |
+++ b/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp |
@@ -143,6 +143,13 @@ static String16 calculateHash(const String16& str) |
return hash.toString(); |
} |
+static bool checkInternalError(ErrorString* errorString, bool success) |
dgozman
2016/03/19 05:39:21
I saw this one already.
kozy
2016/03/21 05:18:28
I've moved it to V8RuntimeAgentImpl. In followup p
|
+{ |
+ if (!success) |
+ *errorString = "Internal error"; |
+ return success; |
+} |
+ |
PassOwnPtr<V8DebuggerAgent> V8DebuggerAgent::create(V8RuntimeAgent* runtimeAgent) |
{ |
V8RuntimeAgentImpl* runtimeAgentImpl = static_cast<V8RuntimeAgentImpl*>(runtimeAgent); |
@@ -708,10 +715,8 @@ void V8DebuggerAgentImpl::getFunctionDetails(ErrorString* errorString, const Str |
scopes = scopesValue.As<v8::Array>(); |
for (size_t i = 0; i < scopes->Length(); ++i) { |
v8::Local<v8::Value> scope; |
- if (!scopes->Get(injectedScript->context(), i).ToLocal(&scope) || !scope->IsObject()) { |
- *errorString = "Internal error"; |
+ if (!checkInternalError(errorString, scopes->Get(injectedScript->context(), i).ToLocal(&scope) && scope->IsObject())) |
return; |
- } |
if (!injectedScript->wrapObjectProperty(errorString, scope.As<v8::Object>(), toV8String(injectedScript->isolate(), "object"), objectGroupName)) |
return; |
} |
@@ -730,10 +735,8 @@ void V8DebuggerAgentImpl::getFunctionDetails(ErrorString* errorString, const Str |
if (!scopes.IsEmpty()) { |
protocol::ErrorSupport errorSupport; |
OwnPtr<protocol::Array<protocol::Debugger::Scope>> scopeChain = protocol::Array<protocol::Debugger::Scope>::parse(toProtocolValue(injectedScript->context(), scopes).get(), &errorSupport); |
- if (errorSupport.hasErrors()) { |
- *errorString = "Internal error"; |
+ if (!checkInternalError(errorString, !errorSupport.hasErrors())) |
dgozman
2016/03/19 05:39:21
Perhaps we should rename this to hasInternalError
kozy
2016/03/21 05:18:28
Done.
|
return; |
- } |
functionDetails->setScopeChain(scopeChain.release()); |
} |
@@ -766,10 +769,8 @@ void V8DebuggerAgentImpl::getGeneratorObjectDetails(ErrorString* errorString, co |
v8::Local<v8::Object> detailsObject; |
v8::Local<v8::Value> detailsValue = debugger().generatorObjectDetails(object); |
- if (!detailsValue->IsObject() || !detailsValue->ToObject(context).ToLocal(&detailsObject)) { |
- *errorString = "Internal error"; |
+ if (!checkInternalError(errorString, detailsValue->IsObject() && detailsValue->ToObject(context).ToLocal(&detailsObject))) |
return; |
- } |
String16 groupName = injectedScript->objectGroupName(*remoteId); |
if (!injectedScript->wrapObjectProperty(errorString, detailsObject, toV8String(m_isolate, "function"), groupName)) |
@@ -777,14 +778,12 @@ void V8DebuggerAgentImpl::getGeneratorObjectDetails(ErrorString* errorString, co |
protocol::ErrorSupport errors; |
OwnPtr<GeneratorObjectDetails> protocolDetails = GeneratorObjectDetails::parse(toProtocolValue(context, detailsObject).get(), &errors); |
- if (!protocolDetails) { |
- *errorString = "Internal error"; |
+ if (!checkInternalError(errorString, protocolDetails)) |
return; |
- } |
*outDetails = protocolDetails.release(); |
} |
-void V8DebuggerAgentImpl::getCollectionEntries(ErrorString* errorString, const String16& objectId, OwnPtr<protocol::Array<CollectionEntry>>* entries) |
+void V8DebuggerAgentImpl::getCollectionEntries(ErrorString* errorString, const String16& objectId, OwnPtr<protocol::Array<CollectionEntry>>* outEntries) |
{ |
if (!checkEnabled(errorString)) |
return; |
@@ -794,7 +793,48 @@ void V8DebuggerAgentImpl::getCollectionEntries(ErrorString* errorString, const S |
InjectedScript* injectedScript = m_injectedScriptManager->findInjectedScript(errorString, remoteId.get()); |
if (!injectedScript) |
return; |
- injectedScript->getCollectionEntries(errorString, objectId, entries); |
+ |
+ v8::HandleScope scope(m_isolate); |
+ v8::Local<v8::Context> context = injectedScript->context(); |
+ v8::Context::Scope contextScope(context); |
+ |
+ v8::Local<v8::Object> object; |
+ v8::Local<v8::Value> value; |
+ if (!injectedScript->findObject(errorString, *remoteId, &value)) |
+ return; |
+ if (!value->IsObject() || !value->ToObject(context).ToLocal(&object)) { |
dgozman
2016/03/19 05:39:21
Use value.As<v8::Object>
kozy
2016/03/21 05:18:28
Done.
|
+ *errorString = "Object with given id is not a collection"; |
+ return; |
+ } |
+ v8::Local<v8::Value> entriesValue = m_debugger->collectionEntries(object); |
+ if (!checkInternalError(errorString, !entriesValue.IsEmpty())) |
+ return; |
+ if (entriesValue->IsUndefined()) { |
+ *errorString = "Object with given id is not a collection"; |
+ return; |
+ } |
+ if (!checkInternalError(errorString, entriesValue->IsArray())) |
+ return; |
+ String16 groupName = injectedScript->objectGroupName(*remoteId); |
+ v8::Local<v8::Array> entriesArray = entriesValue.As<v8::Array>(); |
+ for (size_t i = 0; i < entriesArray->Length(); ++i) { |
+ v8::Local<v8::Value> entry; |
+ if (!checkInternalError(errorString, entriesArray->Get(injectedScript->context(), i).ToLocal(&entry)) && entry->IsObject()) |
dgozman
2016/03/19 05:39:21
entry->IsObject() should be inside checkInternalEr
kozy
2016/03/21 05:18:28
Removed.
|
+ return; |
+ v8::Local<v8::Object> entryObject = entry.As<v8::Object>(); |
+ v8::Local<v8::String> keyString = toV8String(injectedScript->isolate(), "key"); |
dgozman
2016/03/19 05:39:21
toV8StringInternalized
I think there are more lit
kozy
2016/03/21 05:18:28
Done.
|
+ if (entryObject->Has(injectedScript->context(), keyString).FromMaybe(false)) { |
+ if (!injectedScript->wrapObjectProperty(errorString, entryObject, keyString, groupName)) |
+ return; |
+ } |
+ if (!injectedScript->wrapObjectProperty(errorString, entryObject, toV8String(injectedScript->isolate(), "value"), groupName)) |
dgozman
2016/03/19 05:39:21
I'm worried that many calls into V8 like this coul
kozy
2016/03/21 05:18:28
I compare perfomance of different options and intr
|
+ return; |
+ } |
+ protocol::ErrorSupport errors; |
+ OwnPtr<protocol::Array<CollectionEntry>> entries = protocol::Array<CollectionEntry>::parse(toProtocolValue(injectedScript->context(), entriesArray).get(), &errors); |
+ if (!checkInternalError(errorString, entries)) |
dgozman
2016/03/19 05:39:21
I'd rather use !errors.hasErrors()
kozy
2016/03/21 05:18:28
I'd like to check pointer because null pointer wil
|
+ return; |
+ *outEntries = entries.release(); |
} |
void V8DebuggerAgentImpl::schedulePauseOnNextStatement(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data) |