Bug 50019

Summary: Web Inspector: extension API cleanup
Product: WebKit Reporter: Andrey Kosyakov <caseq>
Component: Web Inspector (Deprecated)Assignee: Andrey Kosyakov <caseq>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, apavlov, bweinstein, eric, joepeck, keishi, loislo, pfeldman, pmuellr, rik, sroussey, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
pfeldman: review-
patch
pfeldman: review+
updated API doc none

Description Andrey Kosyakov 2010-11-24 05:15:15 PST
This change renames or removes some method in the WebInspector extension APIs:

- webInspector.log() removed (workaround is to use webInspector.inspectedWindow.eval("consoloe.log(...)", which provides support for different type of console messages and for passing objects)
- InspectorStatus removed (no more calls that may return an error)
- webInspector.inspectedWindow.evaluate() renamed to webInspector.inspectedWindow.eval()
- inspectedWindow.onLoaded and inspectedWindow.onDOMContentLoaded now receive event time in milliseconds since start of main resource event
- callback passed to webInspector.inspectedWindow.eval() now takes two arguments (result, isException), dedicated type is removed
- webInspector.resources.getAll() removed in favor of webInspector.resources.getHAR() (all resources available as har.entries)
- webInspector.resources.get() removed, either use getHAR() or use resources from webInspector.resource.onFinished()
- webInspector.resources.onFinished now takes a single argument, a HAR entry for resource. Resource ID and type are no longer supplied.
- webInspector.resources.getContent() is replaced with getContent method in a resource entry within HAR format
- webInspector.resources.getPageTiming() is removed, use webInspector.resources.getHAR()
- webInspector.panels.scripts getter removed (will be added later)
- ExtensionSidebar.setExpanded() is removed.
- Panel.onSelectionChanged will be panel-specific, currently only supported by Elements panel. No argum,ents are available to ElementsPanel.onSelectionChanged event handler (evaluate $0 within inspected page to get currently inspected node)
- Arguments of webInspector.panels.create() are now in the following order: title, iconURL, pageURL, callback
Comment 1 Andrey Kosyakov 2010-11-24 05:33:27 PST
Created attachment 74751 [details]
patch
Comment 2 Pavel Feldman 2010-11-24 05:48:17 PST
Comment on attachment 74751 [details]
patch

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

> LayoutTests/http/tests/inspector/inspector-test2.js:8
> +    // make sure page-side dispatches are done (e.g. onload in case of reload) and then

This does not sound right. What does it fix?

> LayoutTests/http/tests/inspector/inspector-test2.js:44
>      if (resultsSynchronized)

What is resultsSynchronized?

> LayoutTests/http/tests/inspector/inspector-test2.js:53
> +    function clearResults()

output serves as a log in our tests. I can't imagine why you might want to clear it at runtime.
Comment 3 Andrey Kosyakov 2010-11-24 06:13:05 PST
(In reply to comment #2)
> (From update of attachment 74751 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74751&action=review
> 
> > LayoutTests/http/tests/inspector/inspector-test2.js:8
> > +    // make sure page-side dispatches are done (e.g. onload in case of reload) and then
> 
> This does not sound right. What does it fix?

This ensures we always complete tests _after_ page onload handler is done and the dispatches it produced on front-end are executed. Here's the scenario:

1. Test produces some output with InspectorTest.addResult()
2. Test reloads a page
3. Reloading a page generates some events that are handled by test in front-end and produce additional output
4. Page's onload event is fired
5. runTest() method from insptector-test2.js is executed as a result of (4)
6. pageReloaded handler in front-end is executed as a result of (5)
7. InspectorTest.addResult() synchronizes pre-reload test output to page (see below on that)


> > LayoutTests/http/tests/inspector/inspector-test2.js:44
> >      if (resultsSynchronized)
> What is resultsSynchronized?

This used to be part of inspector-test2 framework for quite some time. The idea is that we loose output as a result of inspected page reload. The code in InspectorTest.addResult() saves all output on front-end and pushes it down to page upon reload (resultsSynchronized is cleared in as a result of page's onload handler).

 
> > LayoutTests/http/tests/inspector/inspector-test2.js:53
> > +    function clearResults()
> 
> output serves as a log in our tests. I can't imagine why you might want to clear it at runtime.

This is triggered after page's onload event, so the output that was written after page loaded but before onload event was fired is duplicated. This change just removes this duplication.
Comment 4 Andrey Kosyakov 2010-11-24 06:32:30 PST
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 74751 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=74751&action=review
> > 
> > > LayoutTests/http/tests/inspector/inspector-test2.js:8
> > > +    // make sure page-side dispatches are done (e.g. onload in case of reload) and then
> > 
> > This does not sound right. What does it fix?
> 
> This ensures we always complete tests _after_ page onload handler is done and the dispatches it produced on front-end are executed. Here's the scenario:
> 
> 1. Test produces some output with InspectorTest.addResult()
> 2. Test reloads a page
> 3. Reloading a page generates some events that are handled by test in front-end and produce additional output
> 4. Page's onload event is fired
> 5. runTest() method from insptector-test2.js is executed as a result of (4)
> 6. pageReloaded handler in front-end is executed as a result of (5)
> 7. InspectorTest.addResult() synchronizes pre-reload test output to page (see below on that)

Sorry, got accidently sent incomplete story. If InspectorTest.completeTest() is called before onload either (5), (6), or (7), the test page will close web inspefctor and call layoutTestController.notifyDone().
Comment 5 Andrey Kosyakov 2010-11-24 07:37:08 PST
Created attachment 74759 [details]
patch

- Removed extra sync in InspectorTest.completeTest() as discussed offline.
Comment 6 Andrey Kosyakov 2010-11-24 10:10:40 PST
Manually committed r72683: http://trac.webkit.org/changeset/72683
Comment 7 Andrey Kosyakov 2010-11-24 10:13:47 PST
Created attachment 74776 [details]
updated API doc
Comment 8 WebKit Review Bot 2010-11-24 11:07:38 PST
http://trac.webkit.org/changeset/72683 might have broken Leopard Intel Release (Tests)
Comment 9 Andrey Kosyakov 2010-11-24 11:25:00 PST
(In reply to comment #8)
> http://trac.webkit.org/changeset/72683 might have broken Leopard Intel Release (Tests)

Test rebaseline landed as r72693: http://trac.webkit.org/changeset/72693