Attachment 342469 Details for Bug 186451 – proposed patch.

[patch] proposed patch.

bug-186451.patch (text/plain), 19.27 KB, created by Mark Lam on 2018-06-11 15:07:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2018-06-11 15:07:01 PDT

Size: 19.27 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 232730)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2018-06-11  Mark Lam  <mark.lam@apple.com>
>+
>+        Add support for webkit-test-runner jscOptions in DumpRenderTree and WebKitTestRunner.
>+        https://bugs.webkit.org/show_bug.cgi?id=186451
>+        <rdar://problem/40875792>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Enhance setOptions() to be able to take a comma separated options string in
>+        addition to white space separated options strings.
>+
>+        * runtime/Options.cpp:
>+        (JSC::isSeparator):
>+        (JSC::Options::setOptions):
>+
> 2018-06-11  Michael Saboff  <msaboff@apple.com>
> 
>         JavaScriptCore: Disable 32-bit JIT on Windows
>Index: Source/JavaScriptCore/runtime/Options.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/Options.cpp	(revision 232730)
>+++ Source/JavaScriptCore/runtime/Options.cpp	(working copy)
>@@ -599,6 +599,11 @@ void Options::dumpOptionsIfNeeded()
>     }
> }
> 
>+static bool isSeparator(char c)
>+{
>+    return isASCIISpace(c) || (c == ',');
>+}
>+
> bool Options::setOptions(const char* optionsStr)
> {
>     Vector<char*> options;
>@@ -609,8 +614,8 @@ bool Options::setOptions(const char* opt
>     char* p = optionsStrCopy;
> 
>     while (p < end) {
>-        // Skip white space.
>-        while (p < end && isASCIISpace(*p))
>+        // Skip separators (white space or commas).
>+        while (p < end && isSeparator(*p))
>             p++;
>         if (p == end)
>             break;
>@@ -637,8 +642,8 @@ bool Options::setOptions(const char* opt
>             hasStringValue = true;
>         }
> 
>-        // Find next white space.
>-        while (p < end && !isASCIISpace(*p))
>+        // Find next separator (white space or commas).
>+        while (p < end && !isSeparator(*p))
>             p++;
>         if (!p)
>             p = end; // No more " " separator. Hence, this is the last arg.
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 232730)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,45 @@
>+2018-06-11  Mark Lam  <mark.lam@apple.com>
>+
>+        Add support for webkit-test-runner jscOptions in DumpRenderTree and WebKitTestRunner.
>+        https://bugs.webkit.org/show_bug.cgi?id=186451
>+        <rdar://problem/40875792>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This jscOptions option can be used by a layout test to specify some JSC runtime
>+        options needed by the test e.g. by adding something like this to the top of the
>+        html file after the DOCTYPE tag:
>+            
>+
>+        If more than one option is needed, the options can be specified as a comma
>+        separated string e.g.
>+            
>+
>+        This only works with JSC options that can be changed at runtime.  Not all JSC
>+        options can be changed this way as some options are only checked and set once at
>+        VM / process initialization time: changing this type of options may have no
>+        effect.  It's the test writer's responsibility to determine which options are
>+        appropriate for with this webkit-test-runner jscOptions option.
>+
>+        This implementation is a workaround until we can change the run-webkit-tests
>+        scripts to parse the option and apply it to a new launch of DRT or WKTR:
>+        https://bugs.webkit.org/show_bug.cgi?id=186452
>+
>+        * DumpRenderTree/TestOptions.cpp:
>+        (TestOptions::TestOptions):
>+        (TestOptions::webViewIsCompatibleWithOptions const):
>+        * DumpRenderTree/TestOptions.h:
>+        * DumpRenderTree/mac/DumpRenderTree.mm:
>+        (setJSCOptions):
>+        (resetWebViewToConsistentStateBeforeTesting):
>+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
>+        (WTR::InjectedBundle::didReceiveMessageToPage):
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::TestController::resetStateToConsistentValues):
>+        (WTR::updateTestOptionsFromTestHeader):
>+        * WebKitTestRunner/TestOptions.h:
>+        (WTR::TestOptions::hasSameInitializationOptions const):
>+
> 2018-06-11  Chris Dumez  <cdumez@apple.com>
> 
>         Allow enabling PSON in layout tests without window.open support
>Index: Tools/DumpRenderTree/TestOptions.cpp
>===================================================================
>--- Tools/DumpRenderTree/TestOptions.cpp	(revision 232730)
>+++ Tools/DumpRenderTree/TestOptions.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2016 Apple Inc. All rights reserved.
>+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -103,11 +103,14 @@ TestOptions::TestOptions(const std::stri
>             enableWebAnimationsCSSIntegration = parseBooleanTestHeaderValue(value);
>         else if (key == "enableColorFilter")
>             enableColorFilter = parseBooleanTestHeaderValue(value);
>+        else if (key == "jscOptions")
>+            jscOptions = value;
>         pairStart = pairEnd + 1;
>     }
> }
> 
> bool TestOptions::webViewIsCompatibleWithOptions(const TestOptions& other) const
> {
>-    return other.layerBackedWebView == layerBackedWebView;
>+    return other.layerBackedWebView == layerBackedWebView
>+        && other.jscOptions == jscOptions;
> }
>Index: Tools/DumpRenderTree/TestOptions.h
>===================================================================
>--- Tools/DumpRenderTree/TestOptions.h	(revision 232730)
>+++ Tools/DumpRenderTree/TestOptions.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2016 Apple Inc. All rights reserved.
>+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -43,6 +43,7 @@ struct TestOptions {
>     bool dumpJSConsoleLogInStdErr { false };
>     bool allowCrossOriginSubresourcesToAskForCredentials { false };
>     bool enableColorFilter { false };
>+    std::string jscOptions;
> 
>     TestOptions(const std::string& pathOrURL, const std::string& absolutePath);
>     bool webViewIsCompatibleWithOptions(const TestOptions&) const;
>Index: Tools/DumpRenderTree/mac/DumpRenderTree.mm
>===================================================================
>--- Tools/DumpRenderTree/mac/DumpRenderTree.mm	(revision 232730)
>+++ Tools/DumpRenderTree/mac/DumpRenderTree.mm	(working copy)
>@@ -57,6 +57,7 @@
> #import "WorkQueue.h"
> #import "WorkQueueItem.h"
> #import <CoreFoundation/CoreFoundation.h>
>+#import <JavaScriptCore/Options.h>
> #import <JavaScriptCore/TestRunnerUtils.h>
> #import <WebCore/LogInitialization.h>
> #import <WebCore/NetworkStorageSession.h>
>@@ -96,6 +97,7 @@
> #import <wtf/ProcessPrivilege.h>
> #import <wtf/RetainPtr.h>
> #import <wtf/Threading.h>
>+#import <wtf/text/StringBuilder.h>
> #import <wtf/text/WTFString.h>
> 
> #if !PLATFORM(IOS)
>@@ -1769,6 +1771,21 @@ static bool shouldMakeViewportFlexible(c
> }
> #endif
> 
>+static void setJSCOptions(const TestOptions& options)
>+{
>+    static WTF::StringBuilder savedOptions;
>+
>+    if (!savedOptions.isEmpty()) {
>+        JSC::Options::setOptions(savedOptions.toString().ascii().data());
>+        savedOptions.clear();
>+    }
>+
>+    if (options.jscOptions.length()) {
>+        JSC::Options::dumpAllOptionsInALine(savedOptions);
>+        JSC::Options::setOptions(options.jscOptions.c_str());
>+    }
>+}
>+
> static void resetWebViewToConsistentStateBeforeTesting(const TestOptions& options)
> {
>     WebView *webView = [mainFrame webView];
>@@ -1848,6 +1865,8 @@ static void resetWebViewToConsistentStat
>     [[NSPasteboard generalPasteboard] declareTypes:[NSArray arrayWithObject:NSStringPboardType] owner:nil];
> #endif
> 
>+    setJSCOptions(options);
>+
>     [mainFrame _clearOpener];
> 
>     setSpellCheckerLoggingEnabled(false);
>Index: Tools/WebKitTestRunner/TestController.cpp
>===================================================================
>--- Tools/WebKitTestRunner/TestController.cpp	(revision 232730)
>+++ Tools/WebKitTestRunner/TestController.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2010, 2014-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -805,6 +805,12 @@ bool TestController::resetStateToConsist
>     }
>     WKDictionarySetItem(resetMessageBody.get(), allowedHostsKey.get(), allowedHostsValue.get());
> 
>+    if (options.jscOptions.length()) {
>+        WKRetainPtr<WKStringRef> jscOptionsKey = adoptWK(WKStringCreateWithUTF8CString("JSCOptions"));
>+        WKRetainPtr<WKStringRef> jscOptionsValue = adoptWK(WKStringCreateWithUTF8CString(options.jscOptions.c_str()));
>+        WKDictionarySetItem(resetMessageBody.get(), jscOptionsKey.get(), jscOptionsValue.get());
>+    }
>+
>     WKPagePostMessageToInjectedBundle(TestController::singleton().mainWebView()->page(), messageName.get(), resetMessageBody.get());
> 
>     WKContextSetShouldUseFontSmoothing(TestController::singleton().context(), false);
>@@ -1079,52 +1085,54 @@ static void updateTestOptionsFromTestHea
>         auto value = pairString.substr(equalsLocation + 1, pairEnd - (equalsLocation + 1));
>         if (key == "language")
>             String(value.c_str()).split(",", false, testOptions.overrideLanguages);
>-        if (key == "useThreadedScrolling")
>+        else if (key == "useThreadedScrolling")
>             testOptions.useThreadedScrolling = parseBooleanTestHeaderValue(value);
>-        if (key == "useAcceleratedDrawing")
>+        else if (key == "useAcceleratedDrawing")
>             testOptions.useAcceleratedDrawing = parseBooleanTestHeaderValue(value);
>-        if (key == "useFlexibleViewport")
>+        else if (key == "useFlexibleViewport")
>             testOptions.useFlexibleViewport = parseBooleanTestHeaderValue(value);
>-        if (key == "useDataDetection")
>+        else if (key == "useDataDetection")
>             testOptions.useDataDetection = parseBooleanTestHeaderValue(value);
>-        if (key == "useMockScrollbars")
>+        else if (key == "useMockScrollbars")
>             testOptions.useMockScrollbars = parseBooleanTestHeaderValue(value);
>-        if (key == "needsSiteSpecificQuirks")
>+        else if (key == "needsSiteSpecificQuirks")
>             testOptions.needsSiteSpecificQuirks = parseBooleanTestHeaderValue(value);
>-        if (key == "ignoresViewportScaleLimits")
>+        else if (key == "ignoresViewportScaleLimits")
>             testOptions.ignoresViewportScaleLimits = parseBooleanTestHeaderValue(value);
>-        if (key == "useCharacterSelectionGranularity")
>+        else if (key == "useCharacterSelectionGranularity")
>             testOptions.useCharacterSelectionGranularity = parseBooleanTestHeaderValue(value);
>-        if (key == "enableAttachmentElement")
>+        else if (key == "enableAttachmentElement")
>             testOptions.enableAttachmentElement = parseBooleanTestHeaderValue(value);
>-        if (key == "enableIntersectionObserver")
>+        else if (key == "enableIntersectionObserver")
>             testOptions.enableIntersectionObserver = parseBooleanTestHeaderValue(value);
>-        if (key == "enableMenuItemElement")
>+        else if (key == "enableMenuItemElement")
>             testOptions.enableMenuItemElement = parseBooleanTestHeaderValue(value);
>-        if (key == "enableModernMediaControls")
>+        else if (key == "enableModernMediaControls")
>             testOptions.enableModernMediaControls = parseBooleanTestHeaderValue(value);
>-        if (key == "enablePointerLock")
>+        else if (key == "enablePointerLock")
>             testOptions.enablePointerLock = parseBooleanTestHeaderValue(value);
>-        if (key == "enableWebAuthentication")
>+        else if (key == "enableWebAuthentication")
>             testOptions.enableWebAuthentication = parseBooleanTestHeaderValue(value);
>-        if (key == "enableIsSecureContextAttribute")
>+        else if (key == "enableIsSecureContextAttribute")
>             testOptions.enableIsSecureContextAttribute = parseBooleanTestHeaderValue(value);
>-        if (key == "enableInspectorAdditions")
>+        else if (key == "enableInspectorAdditions")
>             testOptions.enableInspectorAdditions = parseBooleanTestHeaderValue(value);
>-        if (key == "dumpJSConsoleLogInStdErr")
>+        else if (key == "dumpJSConsoleLogInStdErr")
>             testOptions.dumpJSConsoleLogInStdErr = parseBooleanTestHeaderValue(value);
>-        if (key == "applicationManifest")
>+        else if (key == "applicationManifest")
>             testOptions.applicationManifest = parseStringTestHeaderValueAsRelativePath(value, pathOrURL);
>-        if (key == "allowCrossOriginSubresourcesToAskForCredentials")
>+        else if (key == "allowCrossOriginSubresourcesToAskForCredentials")
>             testOptions.allowCrossOriginSubresourcesToAskForCredentials = parseBooleanTestHeaderValue(value);
>-        if (key == "enableWebAnimationsCSSIntegration")
>+        else if (key == "enableWebAnimationsCSSIntegration")
>             testOptions.enableWebAnimationsCSSIntegration = parseBooleanTestHeaderValue(value);
>-        if (key == "enableProcessSwapOnNavigation")
>+        else if (key == "enableProcessSwapOnNavigation")
>             testOptions.enableProcessSwapOnNavigation = parseBooleanTestHeaderValue(value);
>-        if (key == "enableProcessSwapOnWindowOpen")
>+        else if (key == "enableProcessSwapOnWindowOpen")
>             testOptions.enableProcessSwapOnWindowOpen = parseBooleanTestHeaderValue(value);
>-        if (key == "enableColorFilter")
>+        else if (key == "enableColorFilter")
>             testOptions.enableColorFilter = parseBooleanTestHeaderValue(value);
>+        else if (key == "jscOptions")
>+            testOptions.jscOptions = value;
>         pairStart = pairEnd + 1;
>     }
> }
>Index: Tools/WebKitTestRunner/TestOptions.h
>===================================================================
>--- Tools/WebKitTestRunner/TestOptions.h	(revision 232730)
>+++ Tools/WebKitTestRunner/TestOptions.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2015 Apple Inc. All rights reserved.
>+ * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -63,7 +63,8 @@ struct TestOptions {
>     float deviceScaleFactor { 1 };
>     Vector<String> overrideLanguages;
>     std::string applicationManifest;
>-    
>+    std::string jscOptions;
>+
>     TestOptions(const std::string& pathOrURL);
> 
>     // Add here options that can only be set upon PlatformWebView
>@@ -92,7 +93,8 @@ struct TestOptions {
>             || enableWebAnimationsCSSIntegration != options.enableWebAnimationsCSSIntegration
>             || enableProcessSwapOnNavigation != options.enableProcessSwapOnNavigation
>             || enableProcessSwapOnWindowOpen != options.enableProcessSwapOnWindowOpen
>-            || enableColorFilter != options.enableColorFilter)
>+            || enableColorFilter != options.enableColorFilter
>+            || jscOptions != options.jscOptions)
>             return false;
> 
>         return true;
>Index: Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
>===================================================================
>--- Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp	(revision 232730)
>+++ Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -30,6 +30,7 @@
> #include "InjectedBundlePage.h"
> #include "StringFunctions.h"
> #include "WebCoreTestSupport.h"
>+#include <JavaScriptCore/Options.h>
> #include <WebKit/WKBundle.h>
> #include <WebKit/WKBundlePage.h>
> #include <WebKit/WKBundlePagePrivate.h>
>@@ -221,6 +222,12 @@ void InjectedBundle::didReceiveMessageTo
>         ASSERT(messageBody);
>         ASSERT(WKGetTypeID(messageBody) == WKDictionaryGetTypeID());
>         WKDictionaryRef messageBodyDictionary = static_cast<WKDictionaryRef>(messageBody);
>+        WKRetainPtr<WKStringRef> jscOptionsKey(AdoptWK, WKStringCreateWithUTF8CString("JSCOptions"));
>+        WKRetainPtr<WKStringRef> jscOptionsString = static_cast<WKStringRef>(WKDictionaryGetItemForKey(messageBodyDictionary, jscOptionsKey.get()));
>+        if (jscOptionsString) {
>+            String options = toWTFString(jscOptionsString);
>+            JSC::Options::setOptions(options.utf8().data());
>+        }
> 
>         WKRetainPtr<WKStringRef> shouldGCKey(AdoptWK, WKStringCreateWithUTF8CString("ShouldGC"));
>         bool shouldGC = WKBooleanGetValue(static_cast<WKBooleanRef>(WKDictionaryGetItemForKey(messageBodyDictionary, shouldGCKey.get())));
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 232730)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2018-06-11  Mark Lam  <mark.lam@apple.com>
>+
>+        Add support for webkit-test-runner jscOptions in DumpRenderTree and WebKitTestRunner.
>+        https://bugs.webkit.org/show_bug.cgi?id=186451
>+        <rdar://problem/40875792>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * js/intl-numberformat-format-to-parts.html:
>+        * js/intl-pluralrules.html:
>+        * js/script-tests/intl-numberformat-format-to-parts.js:
>+
> 2018-06-11  Chris Dumez  <cdumez@apple.com>
> 
>         http/tests/security/cors-post-redirect-307.html fails with PSON enabled
>Index: LayoutTests/js/intl-numberformat-format-to-parts.html
>===================================================================
>--- LayoutTests/js/intl-numberformat-format-to-parts.html	(revision 232730)
>+++ LayoutTests/js/intl-numberformat-format-to-parts.html	(working copy)
>@@ -1,4 +1,4 @@
>-<!doctype html>
>+<!doctype html>
> <html>
> <head>
> <meta charset="utf-8">
>Index: LayoutTests/js/intl-pluralrules.html
>===================================================================
>--- LayoutTests/js/intl-pluralrules.html	(revision 232730)
>+++ LayoutTests/js/intl-pluralrules.html	(working copy)
>@@ -1,4 +1,4 @@
>-<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
> <html>
> <head>
> <meta charset="utf-8">
>Index: LayoutTests/js/script-tests/intl-numberformat-format-to-parts.js
>===================================================================
>--- LayoutTests/js/script-tests/intl-numberformat-format-to-parts.js	(revision 232730)
>+++ LayoutTests/js/script-tests/intl-numberformat-format-to-parts.js	(working copy)
>@@ -1,4 +1,6 @@
> //@ skip if $hostOS == "windows" or $hostOS == "darwin" or $hostOS == "linux"
>+//@ requireOptions("--useIntlNumberFormatToParts=true")
>+
> description("This test checks the behavior of Intl.NumberFormat.prototype.formatToParts as described in the ECMAScript Internationalization API Specification.");
> 
> shouldBeType("Intl.NumberFormat.prototype.formatToParts", "Function");

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186451: 342350 | 342469