Bug 113211

Summary: Web Inspector: refactor code duplication: WebInspector.ObjectPropertyTreeElement.wrapPropertyAsElements
Product: WebKit Reporter: Peter Rybin <prybin>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, buildbot, dglazkov, keishi, loislo, pfeldman, pmuellr, rniwa, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from gce-cr-linux-03 for chromium-linux-x86_64
none
Archive of layout-test-results from gce-cr-linux-07 for chromium-linux-x86_64
none
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-04 for mac-future
none
Patch
none
Patch none

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.