Bug 113211 - Web Inspector: refactor code duplication: WebInspector.ObjectPropertyTreeElement.wrapPropertyAsElements
Summary: Web Inspector: refactor code duplication: WebInspector.ObjectPropertyTreeElem...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-25 08:46 PDT by Peter Rybin
Modified: 2013-03-26 10:12 PDT (History)
12 users (show)

See Also:


Attachments
Patch (8.59 KB, patch)
2013-03-25 08:48 PDT, Peter Rybin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-03 for chromium-linux-x86_64 (597.50 KB, application/zip)
2013-03-25 10:11 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from gce-cr-linux-07 for chromium-linux-x86_64 (822.23 KB, application/zip)
2013-03-25 11:05 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (633.40 KB, application/zip)
2013-03-25 12:51 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-future (539.49 KB, application/zip)
2013-03-25 13:04 PDT, Build Bot
no flags Details
Patch (8.64 KB, patch)
2013-03-25 15:17 PDT, Peter Rybin
no flags Details | Formatted Diff | Diff
Patch (8.90 KB, patch)
2013-03-26 09:47 PDT, Peter Rybin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Rybin 2013-03-25 08:46:35 PDT
Currently routine that converts properties into tree elements has copies. This should be merged into a single logic, that would handle accessor properties, function scope pseudo-nodes etc.
Comment 1 Peter Rybin 2013-03-25 08:48:45 PDT
Created attachment 194867 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-25 10:11:28 PDT
Comment on attachment 194867 [details]
Patch

Attachment 194867 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17246424

New failing tests:
inspector/debugger/watch-expressions-panel-switch.html
inspector/elements/event-listener-sidebar.html
inspector/elements/event-listeners-about-blank.html
Comment 3 WebKit Review Bot 2013-03-25 10:11:31 PDT
Created attachment 194878 [details]
Archive of layout-test-results from gce-cr-linux-03 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-03  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment 4 WebKit Review Bot 2013-03-25 11:04:57 PDT
Comment on attachment 194867 [details]
Patch

Attachment 194867 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17211763

New failing tests:
inspector/debugger/watch-expressions-panel-switch.html
inspector/elements/event-listener-sidebar.html
inspector/elements/event-listeners-about-blank.html
Comment 5 WebKit Review Bot 2013-03-25 11:05:01 PDT
Created attachment 194890 [details]
Archive of layout-test-results from gce-cr-linux-07 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-07  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment 6 Build Bot 2013-03-25 12:51:42 PDT
Comment on attachment 194867 [details]
Patch

Attachment 194867 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17313136

New failing tests:
inspector/debugger/watch-expressions-panel-switch.html
inspector/elements/event-listener-sidebar.html
inspector/elements/event-listeners-about-blank.html
Comment 7 Build Bot 2013-03-25 12:51:44 PDT
Created attachment 194906 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 8 Build Bot 2013-03-25 13:04:48 PDT
Comment on attachment 194867 [details]
Patch

Attachment 194867 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17211800

New failing tests:
inspector/debugger/watch-expressions-panel-switch.html
inspector/elements/event-listener-sidebar.html
inspector/elements/event-listeners-about-blank.html
Comment 9 Build Bot 2013-03-25 13:04:51 PDT
Created attachment 194909 [details]
Archive of layout-test-results from webkit-ews-04 for mac-future

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-future  Platform: Mac OS X 10.8.2
Comment 10 Peter Rybin 2013-03-25 15:17:12 PDT
Created attachment 194932 [details]
Patch
Comment 11 Yury Semikhatsky 2013-03-26 05:48:19 PDT
Comment on attachment 194932 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194932&action=review

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:105
>              for (var i = 0; i < this.extraProperties.length; ++i)

style: please add {} for multiline if block.

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:473
> +WebInspector.ObjectPropertyTreeElement.wrapPropertyAsElements = function(treeElement, properties, internalProperties, constructor, comparator, skipProto, value) {

wrapPropertyAsElements -> populateWithProperties
constructor -> treeElementConstructor
Comment 12 Peter Rybin 2013-03-26 09:47:30 PDT
Created attachment 195101 [details]
Patch
Comment 13 Peter Rybin 2013-03-26 09:49:24 PDT
Comment on attachment 194932 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194932&action=review

>> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:105
>>              for (var i = 0; i < this.extraProperties.length; ++i)
> 
> style: please add {} for multiline if block.

Done

>> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:473
>> +WebInspector.ObjectPropertyTreeElement.wrapPropertyAsElements = function(treeElement, properties, internalProperties, constructor, comparator, skipProto, value) {
> 
> wrapPropertyAsElements -> populateWithProperties
> constructor -> treeElementConstructor

Done
Comment 14 WebKit Review Bot 2013-03-26 10:12:17 PDT
Comment on attachment 195101 [details]
Patch

Clearing flags on attachment: 195101

Committed r146900: <http://trac.webkit.org/changeset/146900>
Comment 15 WebKit Review Bot 2013-03-26 10:12:22 PDT
All reviewed patches have been landed.  Closing bug.