Summary: | Web Inspector: brush up object proxies, introduce remote object model. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Feldman <pfeldman> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Pavel Feldman <pfeldman> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, bweinstein, eric, joepeck, keishi, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Pavel Feldman
2010-08-11 05:56:55 PDT
Created attachment 64116 [details]
[PATCH] Proposed change.
Comment on attachment 64116 [details]
[PATCH] Proposed change.
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?
Created attachment 64213 [details]
[PATCH] Review comments addressed.
Comment on attachment 64213 [details]
[PATCH] Review comments addressed.
WebCore/inspector/front-end/ConsoleView.js:593
+ },
please revert InjectedScriptAccess.get change as discussed offline
WebCore/inspector/front-end/EventListenersSidebarPane.js:197
+ value = WebInspector.RemoveObject.fromNode(value.id);
Please fix this line. r- for that
Created attachment 64216 [details]
[PATCH] Review comments addressed.
Committing to http://svn.webkit.org/repository/webkit/trunk ... D WebCore/inspector/front-end/ObjectProxy.js M LayoutTests/ChangeLog M LayoutTests/inspector/console-dir-expected.txt M LayoutTests/inspector/console-dir-global.html M LayoutTests/inspector/console-dir.html M LayoutTests/inspector/console-format-collections-expected.txt M LayoutTests/inspector/console-format-expected.txt M WebCore/ChangeLog M WebCore/WebCore.gypi M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/inspector/front-end/AuditRules.js M WebCore/inspector/front-end/ConsoleView.js M WebCore/inspector/front-end/DOMAgent.js M WebCore/inspector/front-end/ElementsPanel.js M WebCore/inspector/front-end/ElementsTreeOutline.js M WebCore/inspector/front-end/EventListenersSidebarPane.js M WebCore/inspector/front-end/InjectedScript.js M WebCore/inspector/front-end/ObjectPropertiesSection.js M WebCore/inspector/front-end/PropertiesSidebarPane.js M WebCore/inspector/front-end/ScopeChainSidebarPane.js M WebCore/inspector/front-end/ScriptsPanel.js M WebCore/inspector/front-end/SourceFrame.js M WebCore/inspector/front-end/WatchExpressionsSidebarPane.js M WebCore/inspector/front-end/WebKit.qrc M WebCore/inspector/front-end/inspector.html M WebCore/inspector/front-end/inspector.js M WebCore/inspector/front-end/utilities.js Committed r65241 http://trac.webkit.org/changeset/65241 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release Committing to http://svn.webkit.org/repository/webkit/trunk ... D WebCore/inspector/front-end/RemoteObject.js M LayoutTests/ChangeLog M LayoutTests/inspector/console-dir-expected.txt M LayoutTests/inspector/console-dir-global.html M LayoutTests/inspector/console-dir.html M LayoutTests/inspector/console-format-collections-expected.txt M LayoutTests/inspector/console-format-expected.txt M WebCore/ChangeLog M WebCore/WebCore.gypi M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/inspector/front-end/AuditRules.js M WebCore/inspector/front-end/ConsoleView.js M WebCore/inspector/front-end/DOMAgent.js M WebCore/inspector/front-end/ElementsPanel.js M WebCore/inspector/front-end/ElementsTreeOutline.js M WebCore/inspector/front-end/EventListenersSidebarPane.js M WebCore/inspector/front-end/InjectedScript.js M WebCore/inspector/front-end/ObjectPropertiesSection.js A WebCore/inspector/front-end/ObjectProxy.js M WebCore/inspector/front-end/PropertiesSidebarPane.js M WebCore/inspector/front-end/ScopeChainSidebarPane.js M WebCore/inspector/front-end/ScriptsPanel.js M WebCore/inspector/front-end/SourceFrame.js M WebCore/inspector/front-end/WatchExpressionsSidebarPane.js M WebCore/inspector/front-end/WebKit.qrc M WebCore/inspector/front-end/inspector.html M WebCore/inspector/front-end/inspector.js M WebCore/inspector/front-end/utilities.js Committed r65245 If you wish to land manually, webkit-patch mark-bug-fixed will do all the bug updating logic for you. I tthink most people just use webkit-patch land these days though. Are there webkit-patch land issues you've run into? (In reply to comment #9) > If you wish to land manually, webkit-patch mark-bug-fixed will do all the bug updating logic for you. I tthink most people just use webkit-patch land these days though. Are there webkit-patch land issues you've run into? It is mostly speed. I am using git and "git svn rebase" + "git svn dcommit" just works faster for me. Landed as r65248 |