Bug 127448 - Make WebKitTestRunner work with iOS
Summary: Make WebKitTestRunner work with iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad All
: P2 Normal
Assignee: David Farler
URL:
Keywords:
Depends on:
Blocks: 210643
  Show dependency treegraph
 
Reported: 2014-01-22 14:58 PST by David Farler
Modified: 2020-04-17 02:32 PDT (History)
9 users (show)

See Also:


Attachments
Patch (227.00 KB, patch)
2014-01-22 15:03 PST, David Farler
mrowe: review-
Details | Formatted Diff | Diff
Patch (132.93 KB, patch)
2014-01-23 17:14 PST, David Farler
no flags Details | Formatted Diff | Diff
Patch (132.90 KB, patch)
2014-01-24 12:52 PST, David Farler
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Farler 2014-01-22 14:58:43 PST
Make it build - make it launch.
Comment 1 David Farler 2014-01-22 15:03:54 PST
Created attachment 221911 [details]
Patch
Comment 2 Mark Rowe (bdash) 2014-01-22 15:35:24 PST
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.
Comment 3 Csaba Osztrogonác 2014-01-23 03:44:26 PST
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?
Comment 4 Zan Dobersek 2014-01-23 09:13:18 PST
This pretty much looks like a rebase/merge gone wrong, so I don't think there's reason to panic.
Comment 5 David Farler 2014-01-23 11:39:08 PST
Comment on attachment 221911 [details]
Patch

Sorry, this was a botched rebase. I'll repost a clean patch soon.
Comment 6 David Farler 2014-01-23 17:14:37 PST
Created attachment 222044 [details]
Patch
Comment 7 WebKit Commit Bot 2014-01-23 17:15:29 PST
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.
Comment 8 David Farler 2014-01-24 12:52:21 PST
Created attachment 222142 [details]
Patch

More inclusive excluded source files for macosx, make style checker happy
Comment 9 Simon Fraser (smfr) 2014-01-24 13:25:36 PST
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.
Comment 10 David Farler 2014-01-24 15:15:37 PST
Committed r162729: <http://trac.webkit.org/changeset/162729>