[webkit-reviews] review denied: [Bug 43847] Web Inspector: brush up object proxies, introduce remote object model. : [Attachment 64116] [PATCH] Proposed change.

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 43847: Web Inspector: brush up object proxies, introduce remote object
model.
https://bugs.webkit.org/show_bug.cgi?id=43847

Attachment 64116: [PATCH] Proposed change.
https://bugs.webkit.org/attachment.cgi?id=64116&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
WebCore/inspector/front-end/RemoteObject.js:2
 +   * Copyright (C) 2009 Google Inc. All rights reserved.
2009->2010

WebCore/inspector/front-end/RemoteObject.js:55
 +	message._type = "error";
Just pass it as argument to the constructor.

WebCore/inspector/front-end/RemoteObject.js:64
 +  WebInspector.RemoteObject.fromPayload = function(payload)
Maybe fromProtocolMessage or something else more descriptive than just payload?


WebCore/inspector/front-end/RemoteObject.js:68
 +	// FIXME: make sure we only get here with real payloads in the new
DebuggerAgent.js.
There is no DebuggerAgent.js, please change the comment.

WebCore/inspector/front-end/RemoteObject.js:117
 +			result[propertiesPayload[i].name] =
propertiesPayload[i].value.description;
It should return WebInspector.RemoteObjects as values. Otherwise rename the
method to getPropertyValueDescriptions. r- for this.

WebCore/inspector/front-end/PropertiesSidebarPane.js:61
 +	    InjectedScriptAccess.get(-node.id).getPrototypes(node.id,
callback);
Please encapsulate getting injected script id by node id into a method.

WebCore/inspector/front-end/InjectedScript.js:543
 +	    if (typeof obj.length == "number")
== -> ===

WebCore/inspector/front-end/InjectedScript.js:412
 +	var description = InjectedScript._describe(object, abbreviate);
This call was wrapped in try/catch, why did you decide to remove it?