Revert of [inspector] provide more usefull error message for non serializable...

Revert of [inspector] provide more usefull error message for non serializable value (patchset #3 id:40001 of https://codereview.chromium.org/2345263003/ )

Reason for revert:
Breaks https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/9932

See also https://github.com/v8/v8/wiki/Blink-layout-tests

Original issue's description:
> [inspector] provide more usefull error message for non serializable value
>
> Runtime.evaluate can return result by value. We need to provide more details why method call was failed.
>
> BUG=chromium:645640
> R=dgozman@chromium.org,alph@chromium.org
>
> Committed: https://crrev.com/0965b9b5df532d3aa0583966ca60794b54f56943
> Cr-Commit-Position: refs/heads/master@{#39574}

TBR=dgozman@chromium.org,alph@chromium.org,kozyatinskiy@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:645640

Review-Url: https://codereview.chromium.org/2352263003
Cr-Commit-Position: refs/heads/master@{#39580}

src/inspector/InjectedScript.cpp

@@ -143,10 +143,10 @@ void InjectedScript::getProperties(
     *properties = Array<PropertyDescriptor>::create();
     return;
   }
-  if (hasInternalError(errorString, resultValue.IsEmpty())) return;
+
   std::unique_ptr<protocol::Value> protocolValue =
-      toProtocolValue(errorString, context, resultValue);
-  if (!protocolValue) return;
+      toProtocolValue(context, resultValue);
+  if (hasInternalError(errorString, !protocolValue)) return;
   protocol::ErrorSupport errors(errorString);
   std::unique_ptr<Array<PropertyDescriptor>> result =
       Array<PropertyDescriptor>::parse(protocolValue.get(), &errors);
@@ -177,12 +177,10 @@ std::unique_ptr<protocol::Runtime::RemoteObject> InjectedScript::wrapObject(
            .ToLocal(&wrappedObject))
     return nullptr;
   protocol::ErrorSupport errors;
-  std::unique_ptr<protocol::Value> protocolValue =
-      toProtocolValue(errorString, context, wrappedObject);
-  if (!protocolValue) return nullptr;
   std::unique_ptr<protocol::Runtime::RemoteObject> remoteObject =
-      protocol::Runtime::RemoteObject::parse(protocolValue.get(), &errors);
-  if (!remoteObject) *errorString = errors.errors();
+      protocol::Runtime::RemoteObject::parse(
+          toProtocolValue(context, wrappedObject).get(), &errors);
+  if (!remoteObject) *errorString = "Object has too long reference chain";
   return remoteObject;
 }
 
@@ -273,13 +271,10 @@ std::unique_ptr<protocol::Runtime::RemoteObject> InjectedScript::wrapTable(
     function.appendArgument(columns);
   bool hadException = false;
   v8::Local<v8::Value> r = function.call(hadException);
-  if (hadException || r.IsEmpty()) return nullptr;
-  protocol::ErrorString errorString;
-  std::unique_ptr<protocol::Value> protocolValue =
-      toProtocolValue(&errorString, context, r);
-  if (!protocolValue) return nullptr;
+  if (hadException) return nullptr;
   protocol::ErrorSupport errors;
-  return protocol::Runtime::RemoteObject::parse(protocolValue.get(), &errors);
+  return protocol::Runtime::RemoteObject::parse(
+      toProtocolValue(context, r).get(), &errors);
 }
 
 bool InjectedScript::findObject(ErrorString* errorString,

src/inspector/StringUtil.cpp

@@ -103,8 +103,7 @@ std::unique_ptr<protocol::Value> parseJSON(const String16& string) {
 
 }  // namespace protocol
 
-std::unique_ptr<protocol::Value> toProtocolValue(protocol::String* errorString,
-                                                 v8::Local<v8::Context> context,
+std::unique_ptr<protocol::Value> toProtocolValue(v8::Local<v8::Context> context,
                                                  v8::Local<v8::Value> value,
                                                  int maxDepth) {
   if (value.IsEmpty()) {
@@ -112,10 +111,7 @@ std::unique_ptr<protocol::Value> toProtocolValue(protocol::String* errorString,
     return nullptr;
   }
 
-  if (!maxDepth) {
-    *errorString = "Object reference chain is too long";
-    return nullptr;
-  }
+  if (!maxDepth) return nullptr;
   maxDepth--;
 
   if (value->IsNull() || value->IsUndefined()) return protocol::Value::null();
@@ -138,12 +134,9 @@ std::unique_ptr<protocol::Value> toProtocolValue(protocol::String* errorString,
     uint32_t length = array->Length();
     for (uint32_t i = 0; i < length; i++) {
       v8::Local<v8::Value> value;
-      if (!array->Get(context, i).ToLocal(&value)) {
-        *errorString = "Internal error";
-        return nullptr;
-      }
+      if (!array->Get(context, i).ToLocal(&value)) return nullptr;
       std::unique_ptr<protocol::Value> element =
-          toProtocolValue(errorString, context, value, maxDepth);
+          toProtocolValue(context, value, maxDepth);
       if (!element) return nullptr;
       inspectorArray->pushValue(std::move(element));
     }
@@ -154,17 +147,12 @@ std::unique_ptr<protocol::Value> toProtocolValue(protocol::String* errorString,
         protocol::DictionaryValue::create();
     v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(value);
     v8::Local<v8::Array> propertyNames;
-    if (!object->GetPropertyNames(context).ToLocal(&propertyNames)) {
-      *errorString = "Internal error";
+    if (!object->GetPropertyNames(context).ToLocal(&propertyNames))
       return nullptr;
-    }
     uint32_t length = propertyNames->Length();
     for (uint32_t i = 0; i < length; i++) {
       v8::Local<v8::Value> name;
-      if (!propertyNames->Get(context, i).ToLocal(&name)) {
-        *errorString = "Internal error";
-        return nullptr;
-      }
+      if (!propertyNames->Get(context, i).ToLocal(&name)) return nullptr;
       // FIXME(yurys): v8::Object should support GetOwnPropertyNames
       if (name->IsString()) {
         v8::Maybe<bool> hasRealNamedProperty = object->HasRealNamedProperty(
@@ -175,19 +163,16 @@ std::unique_ptr<protocol::Value> toProtocolValue(protocol::String* errorString,
       v8::Local<v8::String> propertyName;
       if (!name->ToString(context).ToLocal(&propertyName)) continue;
       v8::Local<v8::Value> property;
-      if (!object->Get(context, name).ToLocal(&property)) {
-        *errorString = "Internal error";
-        return nullptr;
-      }
+      if (!object->Get(context, name).ToLocal(&property)) return nullptr;
       std::unique_ptr<protocol::Value> propertyValue =
-          toProtocolValue(errorString, context, property, maxDepth);
+          toProtocolValue(context, property, maxDepth);
       if (!propertyValue) return nullptr;
       jsonObject->setValue(toProtocolString(propertyName),
                            std::move(propertyValue));
     }
     return std::move(jsonObject);
   }
-  *errorString = "Object couldn't be returned by value";
+  UNREACHABLE();
   return nullptr;
 }
 

src/inspector/StringUtil.h

@@ -37,8 +37,7 @@ std::unique_ptr<protocol::Value> parseJSON(const String16& json);
 
 }  // namespace protocol
 
-std::unique_ptr<protocol::Value> toProtocolValue(protocol::String* errorString,
-                                                 v8::Local<v8::Context>,
+std::unique_ptr<protocol::Value> toProtocolValue(v8::Local<v8::Context>,
                                                  v8::Local<v8::Value>,
                                                  int maxDepth = 1000);
 

src/inspector/V8DebuggerAgentImpl.cpp

@@ -1012,12 +1012,9 @@ std::unique_ptr<Array<CallFrame>> V8DebuggerAgentImpl::currentCallFrames(
       return Array<CallFrame>::create();
   }
 
-  std::unique_ptr<protocol::Value> protocolValue =
-      toProtocolValue(errorString, debuggerContext, objects);
-  if (!protocolValue) return Array<CallFrame>::create();
   protocol::ErrorSupport errorSupport;
-  std::unique_ptr<Array<CallFrame>> callFrames =
-      Array<CallFrame>::parse(protocolValue.get(), &errorSupport);
+  std::unique_ptr<Array<CallFrame>> callFrames = Array<CallFrame>::parse(
+      toProtocolValue(debuggerContext, objects).get(), &errorSupport);
   if (hasInternalError(errorString, !callFrames))
     return Array<CallFrame>::create();
   return callFrames;