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
Andrey Kosyakov
2010-11-24 05:15:15 PST
Created attachment 74751 [details]
patch
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. (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. (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(). Created attachment 74759 [details]
patch
- Removed extra sync in InspectorTest.completeTest() as discussed offline.
Manually committed r72683: http://trac.webkit.org/changeset/72683 Created attachment 74776 [details]
updated API doc
http://trac.webkit.org/changeset/72683 might have broken Leopard Intel Release (Tests) (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 |