Bug 155295 - [iOS] DumpRenderTree crashes when accessing window.testRunner.inspectorTestStubURL
Summary: [iOS] DumpRenderTree crashes when accessing window.testRunner.inspectorTestSt...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-03-10 03:52 PST by David Kilzer (:ddkilzer)
Modified: 2016-03-10 11:33 PST (History)
7 users (show)

See Also:


Attachments
Patch v1 (6.78 KB, patch)
2016-03-10 04:48 PST, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2016-03-10 03:52:33 PST
DumpRenderTree crashes when accessing window.testRunner.inspectorTestStubURL on these iOS Simulator layout tests:

fast/dom/Window/property-access-on-cached-window-after-frame-removed.html
inspector/debugger/nested-inspectors.html
inspector/debugger/regress-133182.html
inspector/debugger/setBreakpoint-actions.html
inspector/debugger/setBreakpoint-dfg-callee-and-examine-dfg-local.html
inspector/dom/getAccessibilityPropertiesForNode.html
inspector/page/javascriptDialogEvents.html

There are two things that need to be fixed here:

1. Change TestRunner::inspectorTestStubURL() to always return nullptr for PLATFORM(IOS), which matches what we do in WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm.

2. Remove all "inspector/" tests from LayoutTests/platform/ios-simulator-wk{1,2}/TestExpectations.  We already skip "inspector/" and "http/tests/inspector/" tests in LayoutTests/platform/ios-simulator/TestExpectations, so there is no need to resurrect them in the platform-specific TestExpectation files.
Comment 1 David Kilzer (:ddkilzer) 2016-03-10 03:53:30 PST
<rdar://problem/25062709>
Comment 2 David Kilzer (:ddkilzer) 2016-03-10 04:48:10 PST
Created attachment 273562 [details]
Patch v1
Comment 3 Brent Fulgham 2016-03-10 09:34:05 PST
Comment on attachment 273562 [details]
Patch v1

r=me. Is there some other mechanism being used to test iOS inspector features? It does seem like there must be something on the iOS side that should be exercised for inspector, even though the front-end is not used.
Comment 4 David Kilzer (:ddkilzer) 2016-03-10 09:42:25 PST
(In reply to comment #3)
> Comment on attachment 273562 [details]
> Patch v1
> 
> r=me. Is there some other mechanism being used to test iOS inspector
> features? It does seem like there must be something on the iOS side that
> should be exercised for inspector, even though the front-end is not used.

Good question for Web Inspector folks.  I don't think iOS has any (remote) Web Inspector test coverage at the moment.
Comment 5 BJ Burg 2016-03-10 10:15:48 PST
Pasting my comment to Bugzilla too:

The ‘test stub URL’ should be fine to return on iOS in theory, though it might be broken now if the resources are not copied to bundle/private headers for iOS. It’s a fake Inspector Frontend page that’s loaded into an iframe, then the test talks to it as if it were a real out-of-process inspector via postMessage. We call these tests ‘Protocol Tests’ since they don’t use much of the real inspector classes, just testing what comes out of JSC / WebCore.

What we (probably) can’t do on iOS is run the other type of inspector layout tests, which load a full inspector instance into a WebProcess. Pretty sure we just return an error if a local frontend is requested in iOS.

In an ideal world we would have multiple layers of test coverage and the protocol-only ones would cover the frontend-agnostic return values of inspector backend commands, and run on both iOS and Mac. We are definitely not there yet since it’s a lot of work to convert old tests.
Comment 6 WebKit Commit Bot 2016-03-10 10:23:41 PST
Comment on attachment 273562 [details]
Patch v1

Clearing flags on attachment: 273562

Committed r197942: <http://trac.webkit.org/changeset/197942>
Comment 7 WebKit Commit Bot 2016-03-10 10:23:46 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 David Kilzer (:ddkilzer) 2016-03-10 10:37:19 PST
(In reply to comment #5)
> The ‘test stub URL’ should be fine to return on iOS in theory, though it
> might be broken now if the resources are not copied to bundle/private
> headers for iOS. It’s a fake Inspector Frontend page that’s loaded into an
> iframe, then the test talks to it as if it were a real out-of-process
> inspector via postMessage. We call these tests ‘Protocol Tests’ since they
> don’t use much of the real inspector classes, just testing what comes out of
> JSC / WebCore.

WebInspectorUI.framework is not built for iOS, so it doesn't exist when DumpRenderTree/WebKitTestRunner tries to load the HTML file from that bundle.

Maybe it should move into Tools somewhere?  Or JavaScriptCore.framework or WebCore.framework?  Or should we build WebInspectorUI.framework for testing on iOS only?
Comment 9 BJ Burg 2016-03-10 11:33:56 PST
(In reply to comment #8)
> (In reply to comment #5)
> > The ‘test stub URL’ should be fine to return on iOS in theory, though it
> > might be broken now if the resources are not copied to bundle/private
> > headers for iOS. It’s a fake Inspector Frontend page that’s loaded into an
> > iframe, then the test talks to it as if it were a real out-of-process
> > inspector via postMessage. We call these tests ‘Protocol Tests’ since they
> > don’t use much of the real inspector classes, just testing what comes out of
> > JSC / WebCore.
> 
> WebInspectorUI.framework is not built for iOS, so it doesn't exist when
> DumpRenderTree/WebKitTestRunner tries to load the HTML file from that bundle.
> 
> Maybe it should move into Tools somewhere?  Or JavaScriptCore.framework or
> WebCore.framework?  Or should we build WebInspectorUI.framework for testing
> on iOS only?

The last option sounds easiest to me. The various test runner files change quite a bit and putting them in JSC / WebCore would be a few steps backward in hackability (not to mention the other problems re: minifying and versioning)