Bug 127448 – Make WebKitTestRunner work with iOS

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Farler]

Make it build - make it launch.

[David Farler]

Created attachment 221911 [details]
Patch

[Mark Rowe (bdash)]

Comment on attachment 221911 [details]
Patch

I'm not sure why this changing a bunch of ATK code. It doesn't seem related to iOS at all.

[Csaba Osztrogonác]

Comment on attachment 221911 [details]
Patch

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

> Tools/WebKitTestRunner/GNUmakefile.am:65
> +	-no-fast-install \

How is it related to iOS?

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityControllerAtk.cpp:65
> -    m_globalNotificationHandler = nullptr;
> +    m_globalNotificationHandler = 0;

Why do you use 0 instead of nullptr? c++11 is mandatory now for all platform.

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityControllerAtk.cpp:71
> -        return nullptr;
> +        return 0;

ditto

> Tools/WebKitTestRunner/TestController.cpp:-271
> -    Options options(defaultLongTimeout, defaultShortTimeout);
> -    OptionsHandler optionsHandler(options);

Please don't revert http://trac.webkit.org/changeset/160627 to make it work on iOS

> Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp:58
> -    MD5::Digest hash;
> +    Vector<uint8_t, 16> hash;

ditto

> Tools/WebKitTestRunner/cg/TestInvocationCG.cpp:111
> -
> +    
>      hashString[0] = '\0';
> -    for (size_t i = 0; i < MD5::hashSize; i++)
> +    for (int i = 0; i < 16; i++)

Please don't revert http://trac.webkit.org/changeset/160386

> Tools/WebKitTestRunner/efl/TestControllerEfl.cpp:106
> -void TestController::setHidden(bool hidden)
> +void TestController::setHidden(bool)
>  {
> -    PlatformWKView view = mainWebView()->platformView();
> -
> -    if (!view) {
> -        fprintf(stderr, "ERROR: view is null.\n");
> -        return;
> -    }
> -
> -    if (hidden)
> -        evas_object_hide(view);
> -    else
> -        evas_object_show(view);
> +    // FIXME: Need to implement this to test visibilityState.

Why did you remove this?

[Zan Dobersek]

This pretty much looks like a rebase/merge gone wrong, so I don't think there's reason to panic.

[David Farler]

Comment on attachment 221911 [details]
Patch

Sorry, this was a botched rebase. I'll repost a clean patch soon.

[David Farler]

Created attachment 222044 [details]
Patch

[WebKit Commit Bot]

Attachment 222044 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:17:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Tools/ChangeLog:18:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Tools/ChangeLog:19:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Tools/ChangeLog:21:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 4 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Farler]

Created attachment 222142 [details]
Patch

More inclusive excluded source files for macosx, make style checker happy

[Simon Fraser (smfr)]

Comment on attachment 222142 [details]
Patch

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

> Tools/WebKitTestRunner/InjectedBundle/ios/ActivateFontsIOS.mm:2
> + * Copyright (C) 2013 Apple Inc. All rights reserved.

These should all be 2014

> Tools/WebKitTestRunner/TestController.cpp:315
> +#if !PLATFORM(IOS)
>  #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED > 1080

I think we do want this now.

[David Farler]

Committed r162729: <http://trac.webkit.org/changeset/162729>