Bug 42049 – Implement waitUntilDone and notifyDone for WebKitTestRunner

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Maciej Stachowiak]

Implement waitUntilDone and notifyDone for WebKitTestRunner

[Maciej Stachowiak]

Created attachment 61187 [details]
Patch

Comment on attachment 61187 [details]
Patch

> +static const CFTimeInterval waitToDumpWatchdogInterval = 30.0;

Current style is to omit the “.0”.

> +void LayoutTestController::setWaitToDump(bool waitUntilDone)
> +{
> +    m_waitToDump = waitUntilDone;
> +    if (m_waitToDump && !m_waitToDumpWatchdog) {
> +        m_waitToDumpWatchdog.adoptCF(CFRunLoopTimerCreate(kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + waitToDumpWatchdogInterval, 
> +                                                      0, 0, 0, waitUntilDoneWatchdogFired, NULL));
> +        CFRunLoopAddTimer(CFRunLoopGetCurrent(), m_waitToDumpWatchdog.get(), kCFRunLoopCommonModes);
> +    }
> +}

It’s not clear that this method does the right thing when waitUntilDone is false. Fortunately, it’s always passed true. (In particular, if it were ever possible for this to get called with false, wouldn’t it need to invalidate the watchdog timer and dump if the page has already finished loading?). Anyway, this is the same in DRT.

> +static JSValueRef waitUntilDoneCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
> +{
> +    // Has mac & windows implementation
> +    LayoutTestController* controller = static_cast<LayoutTestController*>(JSObjectGetPrivate(thisObject));
> +    controller->setWaitToDump(true);
> +
> +    return JSValueMakeUndefined(context);
> +}
> +
> +static JSValueRef notifyDoneCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
> +{
> +    LayoutTestController* controller = static_cast<LayoutTestController*>(JSObjectGetPrivate(thisObject));
> +    controller->notifyDone();
> +    return JSValueMakeUndefined(context);
> +}

I am not sure why one of these deserves a comment and an empty line before the return statement and the other doesn’t.

> +    bool m_waitToDump; // True if waitUntilDone() has been called, but notifyDone() has not yet been called.

Shouldn’t this member variable be initialized to false in the constructor?

Comment on attachment 61187 [details]
Patch

r+ based on changes Maciej said he’d make before landing

[Maciej Stachowiak]

Created attachment 61188 [details]
Patch

Comment on attachment 61188 [details]
Patch


> +
> [phone]  Maciej Stachowiak  <[e-mail]>
> +
> +        * WebKitTestRunner/InjectedBundle/LayoutTestController.cpp:
> +

Stray changelog entry.

> +static const CFTimeInterval waitToDumpWatchdogInterval = 30.0;

Current style is to omit the “.0”.

r=me

[Maciej Stachowiak]

Committed r63061: <http://trac.webkit.org/changeset/63061>