Bug 129190 - Web Inspector: model tests should use a special Test.html inspector page
Summary: Web Inspector: model tests should use a special Test.html inspector page
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks: 129217
  Show dependency treegraph
 
Reported: 2014-02-21 18:10 PST by BJ Burg
Modified: 2014-02-27 14:02 PST (History)
11 users (show)

See Also:


Attachments
the patch (55.81 KB, patch)
2014-02-25 18:23 PST, BJ Burg
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (555.06 KB, application/zip)
2014-02-26 00:50 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (583.73 KB, application/zip)
2014-02-26 07:15 PST, Build Bot
no flags Details
patch, now with less fail (55.81 KB, patch)
2014-02-26 12:58 PST, BJ Burg
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (500.31 KB, application/zip)
2014-02-26 14:00 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (554.34 KB, application/zip)
2014-02-26 14:31 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (574.17 KB, application/zip)
2014-02-26 15:47 PST, Build Bot
no flags Details
patch, now with even less fail (56.85 KB, patch)
2014-02-27 10:21 PST, BJ Burg
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2014-02-21 18:10:19 PST
Loading the entire Main.html page for replay and model tests is overkill and will make the tests too slow. Instead we should load a separate Test.html (which could be generated) that only loads models and controllers.

For WebKit2, we can create a new WebInspector message called createInspectorPageForTest() which navigates the page to a different URL. This can be called by the WK2 inspector client whenever InspectorController::isUnderTest() is true, so subsequent calls to showWebInspector() from the test runner just do the right thing.

For WebKit1, we can change the logic in WebInspectorClient::openInspectorFrontend implementations to load a different URL according to the passed-in InspectorController, or we can ask the InspectorController what page to load. A naive implementation might not play nice with bundles and frameworks, however.
Comment 1 Radar WebKit Bug Importer 2014-02-21 18:10:54 PST
<rdar://problem/16140738>
Comment 2 Timothy Hatcher 2014-02-22 16:40:11 PST
This should be easy no that the UserInterface folder is segregated.
Comment 3 BJ Burg 2014-02-25 18:23:02 PST
Created attachment 225208 [details]
the patch
Comment 4 WebKit Commit Bot 2014-02-25 18:24:43 PST
Attachment 225208 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/WebPage/WebInspector.cpp:111:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 BJ Burg 2014-02-25 18:27:35 PST
Unfortunately the approach of resetting internals to a consistent state when first injecting it is probably responsible for a lot of test failures. I may have to make a separate method to do that.
Comment 6 Build Bot 2014-02-26 00:50:27 PST
Comment on attachment 225208 [details]
the patch

Attachment 225208 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5620999726825472

New failing tests:
media/track/track-css-user-override.html
fast/canvas/webgl/webgl-reload-crash.html
fast/profiler/multiple-frames.html
fast/frames/flattening/iframe-flattening-crash.html
http/tests/security/xssAuditor/get-from-iframe.html
http/tests/webgl/1.0.2/texImage2DHTML.html
fast/frames/flattening/frameset-flattening-subframe-resize.html
fast/forms/basic-textareas.html
fast/images/exif-orientation.html
inspector-protocol/dom/dom-search-crash.html
fast/spatial-navigation/snav-iframe-flattening-simple.html
http/tests/security/xssAuditor/xss-protection-parsing-01.html
http/tests/security/xssAuditor/cached-frame.html
fast/events/popup-blocked-from-iframe-script.html
fast/canvas/webgl/context-release-upon-reload.html
http/tests/security/xssAuditor/full-block-get-from-iframe.html
platform/mac/accessibility/iframe-aria-hidden.html
fast/spatial-navigation/snav-iframe-nested.html
fast/spatial-navigation/snav-iframe-with-offscreen-focusable-element.html
svg/masking/mask-negative-scale.svg
http/tests/security/xssAuditor/post-from-iframe.html
fast/spatial-navigation/snav-search-optimization.html
fast/parser/pre-html5-parser-quirks.html
http/tests/security/xssAuditor/full-block-post-from-iframe.html
fast/spatial-navigation/snav-iframe-no-scrollable-content.html
http/tests/webgl/1.0.2/texSubImage2DHTML.html
fast/spatial-navigation/snav-iframe-recursive-offset-parent.html
fast/events/popup-blocked-from-iframe-src.html
fast/dom/HTMLDocument/hasFocus.html
fast/spatial-navigation/snav-iframe-no-focusable-content.html
Comment 7 Build Bot 2014-02-26 00:50:31 PST
Created attachment 225236 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Build Bot 2014-02-26 07:15:35 PST
Comment on attachment 225208 [details]
the patch

Attachment 225208 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5314270443077632

New failing tests:
media/track/track-css-user-override.html
webgl/1.0.1/conformance/more/functions/bufferDataBadArgs.html
webgl/1.0.1/conformance/more/conformance/quickCheckAPI-A.html
fast/profiler/multiple-frames.html
webgl/1.0.1/conformance/more/functions/bindBufferBadArgs.html
webgl/1.0.1/conformance/more/conformance/quickCheckAPI-B1.html
fast/frames/flattening/iframe-flattening-crash.html
webgl/1.0.1/conformance/more/functions/copyTexImage2DBadArgs.html
webgl/1.0.1/conformance/more/conformance/quickCheckAPI-C.html
fast/frames/flattening/frameset-flattening-subframe-resize.html
fast/forms/basic-textareas.html
webgl/1.0.1/conformance/more/functions/drawArrays.html
webgl/1.0.1/conformance/more/conformance/quickCheckAPI-S_V.html
http/tests/security/xssAuditor/xss-protection-parsing-01.html
http/tests/security/xssAuditor/cached-frame.html
fast/events/popup-blocked-from-iframe-script.html
svg/masking/mask-negative-scale.svg
http/tests/security/xssAuditor/full-block-get-from-iframe.html
webgl/1.0.1/conformance/more/functions/drawArraysOutOfBounds.html
platform/mac/accessibility/iframe-aria-hidden.html
webgl/1.0.1/conformance/more/conformance/quickCheckAPI-G_I.html
webgl/1.0.1/conformance/more/functions/bufferSubDataBadArgs.html
http/tests/security/xssAuditor/post-from-iframe.html
http/tests/security/xssAuditor/full-block-post-from-iframe.html
http/tests/security/xssAuditor/get-from-iframe.html
webgl/1.0.1/conformance/glsl/misc/uniform-location-length-limits.html
webgl/1.0.1/conformance/more/conformance/quickCheckAPI-D_G.html
webgl/1.0.1/conformance/more/conformance/quickCheckAPI-B2.html
fast/events/popup-blocked-from-iframe-src.html
webgl/1.0.1/conformance/more/functions/copyTexSubImage2DBadArgs.html
Comment 9 Build Bot 2014-02-26 07:15:40 PST
Created attachment 225253 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 10 BJ Burg 2014-02-26 12:58:19 PST
Created attachment 225286 [details]
patch, now with less fail
Comment 11 WebKit Commit Bot 2014-02-26 12:59:58 PST
Attachment 225286 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/WebPage/WebInspector.cpp:111:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Build Bot 2014-02-26 14:00:24 PST
Comment on attachment 225286 [details]
patch, now with less fail

Attachment 225286 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6246613890629632

New failing tests:
media/track/track-css-user-override.html
fast/profiler/multiple-frames.html
platform/mac/accessibility/iframe-aria-hidden.html
Comment 13 Build Bot 2014-02-26 14:00:27 PST
Created attachment 225293 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 14 Build Bot 2014-02-26 14:31:28 PST
Comment on attachment 225286 [details]
patch, now with less fail

Attachment 225286 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5383906895331328

New failing tests:
media/track/track-css-user-override.html
svg/masking/mask-negative-scale.svg
fast/profiler/multiple-frames.html
platform/mac/accessibility/iframe-aria-hidden.html
Comment 15 Build Bot 2014-02-26 14:31:30 PST
Created attachment 225300 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 16 Build Bot 2014-02-26 15:47:10 PST
Comment on attachment 225286 [details]
patch, now with less fail

Attachment 225286 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6255068533751808

New failing tests:
media/track/track-css-user-override.html
platform/mac/accessibility/iframe-aria-hidden.html
fast/profiler/multiple-frames.html
Comment 17 Build Bot 2014-02-26 15:47:14 PST
Created attachment 225313 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 18 BJ Burg 2014-02-26 15:51:17 PST
This patch is ready to be reviewed. There are 3-4 remaining test failures, and I would appreciate any help diagnosing. Some of them (WebGL, etc) seem to be fixed on ToT.
Comment 19 BJ Burg 2014-02-27 10:21:26 PST
Created attachment 225391 [details]
patch, now with even less fail
Comment 20 WebKit Commit Bot 2014-02-27 10:23:37 PST
Attachment 225391 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/WebPage/WebInspector.cpp:111:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 1 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Timothy Hatcher 2014-02-27 11:12:29 PST
Comment on attachment 225391 [details]
patch, now with even less fail

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

> Source/WebInspectorUI/UserInterface/Base/Test.js:28
> +WebInspector.loaded = function()
> +{
> +    // Register observers for events from the InspectorBackend.

A comment about matching Main.js order?

> Source/WebInspectorUI/UserInterface/Protocol/InspectorObserver.js:36
> +    // FIXME: remove plumbing for callIdentifier parameter from backend, as it's not used.

Nit: remove should be Remove

> Source/WebInspectorUI/UserInterface/Test.html:33
> +    <!--
> +    Each subsection is alphabetical, with the exception that base classes must
> +    be loaded before any classes that inherit from the base class are loaded.
> +    -->

This is correct. But just say 'match Main.html order/grouping'? Note: I left a similar comment out of Main.html to prevent it from shipping with a comment.

> Source/WebInspectorUI/UserInterface/Test.html:109
> +<div id="docked-resizer"></div>
> +<div id="toolbar"></div>
> +<div id="main">
> +    <div id="navigation-sidebar"></div>
> +    <div id="content">
> +        <div id="content-browser"></div>
> +        <div id="split-content-browser" class="hidden"></div>
> +        <div id="quick-console"></div>
> +    </div>
> +    <div id="details-sidebar"></div>
> +</div>

None of this should be needed. Right?

> LayoutTests/http/tests/inspector-protocol/resources/InspectorTest.js:161
> +// FIXME: move model tests off of the stub inspector page, and delete this function

Nit: move should be Move
Comment 22 BJ Burg 2014-02-27 12:39:46 PST
Comment on attachment 225391 [details]
patch, now with even less fail

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

>> Source/WebInspectorUI/UserInterface/Base/Test.js:28
>> +    // Register observers for events from the InspectorBackend.
> 
> A comment about matching Main.js order?

OK
Comment 23 BJ Burg 2014-02-27 14:02:37 PST
Committed r164830: <http://trac.webkit.org/changeset/164830>