Bug 149356

Summary: Add the ability for tests to run script in the UI process in WebKitTestRunner
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, buildbot, commit-queue, dbates, enrica, gyuyoung.kim, rniwa, simon.fraser, thorton
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
For EWS
none
For EWS
none
Try to fix Efl build
none
Archive of layout-test-results from ews103 for mac-mavericks
none
For EWS
none
Patch for EWS simon.fraser: review+, commit-queue: commit-queue-

Description Simon Fraser (smfr) 2015-09-18 15:28:53 PDT
Add the ability for tests to run script in the UI process in WebKitTestRunner
Comment 1 Simon Fraser (smfr) 2015-09-18 16:03:41 PDT
Created attachment 261537 [details]
Patch
Comment 2 Tim Horton 2015-09-18 16:28:28 PDT
Comment on attachment 261537 [details]
Patch

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

> Tools/WebKitTestRunner/DerivedSources.make:26
> +    $(WebKitTestRunner)/uiscriptcontext/bindings \

Capitalization? I think we tend to camel-case directory names. UIScriptContext/Bindings?

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:244
> +        unsigned callbackID = (int)WKUInt64GetValue(static_cast<WKUInt64Ref>(WKDictionaryGetItemForKey(messageBodyDictionary, callbackIDKey.get())));

unsigned = (int)uint64_t. 

why?

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:247
> +        JSStringRef resultJSString = WKStringCopyJSString(resultString);

JSRetainPtr?

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:900
> +    // install callback

probably not necessary, and if it is it should be capitalized/etc.

> Tools/WebKitTestRunner/TestInvocation.cpp:152
> +bool TestInvocation::shouldUseFixedLayout() const

?

> Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:49
> +#if !PLATFORM(IOS)

PLATFORM(MAC). This isn't about not being on iOS.

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:85
> +    WKRect wkRect;

WKRectMake?

> Tools/WebKitTestRunner/mac/WebKitTestRunnerDraggingInfo.h:26
> +#if !PLATFORM(IOS)

What I said before, I think?

> Tools/WebKitTestRunner/uiscriptcontext/UIScriptContext.cpp:40
> +    m_context = JSGlobalContextCreate(nullptr);

JSRetainPtr?

> Tools/WebKitTestRunner/uiscriptcontext/UIScriptContext.cpp:58
> +    JSStringRef scriptRef = WKStringCopyJSString(script);

ditto, etc.

> LayoutTests/TestExpectations:39
> +# Zooming is iOS-specific

It's not, really.

> LayoutTests/TestExpectations:40
> +fast/zooming [ Skip ]

Didn't we say fast/zooming/ios?
Comment 3 Simon Fraser (smfr) 2015-09-18 17:38:56 PDT
Created attachment 261548 [details]
For EWS
Comment 4 Simon Fraser (smfr) 2015-09-18 18:02:21 PDT
Created attachment 261549 [details]
For EWS
Comment 5 Simon Fraser (smfr) 2015-09-18 18:20:38 PDT
Created attachment 261552 [details]
Try to fix Efl build
Comment 6 Build Bot 2015-09-18 18:56:45 PDT
Comment on attachment 261552 [details]
Try to fix Efl build

Attachment 261552 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/184969

New failing tests:
fast/harness/concurrent-ui-side-scripts.html
fast/harness/ui-side-scripts.html
Comment 7 Build Bot 2015-09-18 18:56:50 PDT
Created attachment 261558 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 8 Simon Fraser (smfr) 2015-09-18 20:45:50 PDT
Created attachment 261566 [details]
For EWS
Comment 9 Darin Adler 2015-09-19 15:51:11 PDT
Comment on attachment 261566 [details]
For EWS

Build failures are:

UIScriptContext.cpp.o:UIScriptContext.cpp:function WTR::UIScriptContext::UIScriptContext(WTR::UIScriptContextDelegate&): error: undefined reference to 'WTR::UIScriptController::makeWindowObject(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue const**)'
Comment 10 Simon Fraser (smfr) 2015-09-19 18:47:36 PDT
Yeah, I need help with the CMake stuff for Efl/Gtk.
Comment 11 Gyuyoung Kim 2015-09-21 08:23:45 PDT
Created attachment 261658 [details]
Patch for EWS

I'm not sure if this is correct fix yet. However I'd like to check if this patch can pass both EFL and GTK ports.
Comment 12 Simon Fraser (smfr) 2015-09-21 09:55:50 PDT
Comment on attachment 261658 [details]
Patch for EWS

Patch seems to work! Can I land it?
Comment 13 WebKit Commit Bot 2015-09-21 11:11:05 PDT
Comment on attachment 261658 [details]
Patch for EWS

Rejecting attachment 261658 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 261658, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
tching file Tools/WebKitTestRunner/mac/TestControllerMac.mm
Hunk #2 FAILED at 78.
Hunk #3 FAILED at 93.
2 out of 3 hunks FAILED -- saving rejects to file Tools/WebKitTestRunner/mac/TestControllerMac.mm.rej
patching file Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm
patching file Tools/WebKitTestRunner/mac/WebKitTestRunnerDraggingInfo.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Simon Fraser']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/193899
Comment 14 Simon Fraser (smfr) 2015-09-21 12:11:38 PDT
https://trac.webkit.org/r190065
Comment 15 Ryosuke Niwa 2015-09-21 12:52:12 PDT
This broke 32-bit builds:
https://build.webkit.org/builders/Apple%20Yosemite%20Release%20%2832-bit%20Build%29/builds/6158

/Volumes/Data/slave/yosemite-32bit-release/build/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:33:12: error: cannot find interface declaration for 'WKWebView'; did you mean 'WebView'?
Comment 16 Simon Fraser (smfr) 2015-09-21 13:34:34 PDT
I'll fix soon.
Comment 17 Ryosuke Niwa 2015-09-21 13:46:03 PDT
Fixed it in https://trac.webkit.org/changeset/190075.