[DRT] TestOptions should not be ObjC.

https://bugs.webkit.org/show_bug.cgi?id=183487

Reviewed by Per Arne Vollan.

* DumpRenderTree/CMakeLists.txt:
* DumpRenderTree/DumpRenderTree.xcodeproj/project.pbxproj:
* DumpRenderTree/PlatformMac.cmake:
Move TestOptions to platform-agnostic sources.

* DumpRenderTree/TestOptions.h:
* DumpRenderTree/TestOptions.cpp: Renamed from Tools/DumpRenderTree/TestOptions.mm.
* DumpRenderTree/mac/DumpRenderTree.mm:
Remove NSURL dependency and align with WTR as much as possible.

* DumpRenderTree/win/DumpRenderTree.cpp:
Consume TestOptions (recognizing just the ones that have WebPreferences defined for now).


Canonical link: https://commits.webkit.org/199252@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229565 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog

@@ -1,3 +1,23 @@
+2018-03-12  Ross Kirsling  <ross.kirsling@sony.com>
+
+        [DRT] TestOptions should not be ObjC.
+        https://bugs.webkit.org/show_bug.cgi?id=183487
+
+        Reviewed by Per Arne Vollan.
+
+        * DumpRenderTree/CMakeLists.txt:
+        * DumpRenderTree/DumpRenderTree.xcodeproj/project.pbxproj:
+        * DumpRenderTree/PlatformMac.cmake:
+        Move TestOptions to platform-agnostic sources.
+
+        * DumpRenderTree/TestOptions.h:
+        * DumpRenderTree/TestOptions.cpp: Renamed from Tools/DumpRenderTree/TestOptions.mm.
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        Remove NSURL dependency and align with WTR as much as possible.
+
+        * DumpRenderTree/win/DumpRenderTree.cpp:
+        Consume TestOptions (recognizing just the ones that have WebPreferences defined for now).
+
 2018-03-12  Yoav Weiss  <yoav@yoav.ws>
 
         Runtime flag for link prefetch and remove link subresource.

Tools/DumpRenderTree/CMakeLists.txt

@@ -13,6 +13,7 @@ set(DumpRenderTree_SOURCES
     GCController.cpp
     JavaScriptThreading.cpp
     PixelDumpSupport.cpp
+    TestOptions.cpp
     TestRunner.cpp
     WorkQueue.cpp
     ${WEBKIT_TESTRUNNER_UISCRIPTCONTEXT_DIR}/UIScriptContext.cpp

Tools/DumpRenderTree/DumpRenderTree.xcodeproj/project.pbxproj

@@ -101,14 +101,14 @@
 		5DB9AC9E0F722C3600684641 /* WebKitWeightWatcher700.ttf in Copy Font Files */ = {isa = PBXBuildFile; fileRef = 375F09770DAC3CB600C8B4E5 /* WebKitWeightWatcher700.ttf */; };
 		5DB9AC9F0F722C3600684641 /* WebKitWeightWatcher800.ttf in Copy Font Files */ = {isa = PBXBuildFile; fileRef = 375F09780DAC3CB600C8B4E5 /* WebKitWeightWatcher800.ttf */; };
 		5DB9ACA00F722C3600684641 /* WebKitWeightWatcher900.ttf in Copy Font Files */ = {isa = PBXBuildFile; fileRef = 375F09790DAC3CB600C8B4E5 /* WebKitWeightWatcher900.ttf */; };
-		7CBBC3231DDFCF9A00786B9D /* TestOptions.mm in Sources */ = {isa = PBXBuildFile; fileRef = 7CBBC3211DDFCF9A00786B9D /* TestOptions.mm */; };
 		80045AEE147718E7008290A8 /* AccessibilityNotificationHandler.mm in Sources */ = {isa = PBXBuildFile; fileRef = 80045AEC147718E7008290A8 /* AccessibilityNotificationHandler.mm */; };
 		8465E2C70FFA8DF2003B8342 /* PixelDumpSupport.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8465E2C60FFA8DF2003B8342 /* PixelDumpSupport.cpp */; };
 		9830F31F15C81181005AB206 /* DumpRenderTreeCommon.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9830F31E15C81181005AB206 /* DumpRenderTreeCommon.cpp */; };
 		A1158D581892740C0088C17B /* DumpRenderTreeBrowserView.mm in Sources */ = {isa = PBXBuildFile; fileRef = A1158D56189273EB0088C17B /* DumpRenderTreeBrowserView.mm */; };
 		A1158D59189274360088C17B /* PixelDumpSupportIOS.mm in Sources */ = {isa = PBXBuildFile; fileRef = A1158D57189273EB0088C17B /* PixelDumpSupportIOS.mm */; };
 		A134E531188FC27000901D06 /* DumpRenderTreeMain.mm in Sources */ = {isa = PBXBuildFile; fileRef = A134E52F188FC27000901D06 /* DumpRenderTreeMain.mm */; };
 		A134E53618905EFF00901D06 /* AccessibilityCommonMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = BC0E26140E2DA4C6001B6BC3 /* AccessibilityCommonMac.mm */; };
+		A30A21F82051D8C40008FF42 /* TestOptions.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A30A21F72051D8960008FF42 /* TestOptions.cpp */; };
 		A8B91ADA0CF3B32F008F91FF /* DumpRenderTreePasteboard.mm in Sources */ = {isa = PBXBuildFile; fileRef = A8B91AD70CF3B32F008F91FF /* DumpRenderTreePasteboard.mm */; };
 		A8B91ADC0CF3B32F008F91FF /* DumpRenderTreeWindow.mm in Sources */ = {isa = PBXBuildFile; fileRef = A8B91AD90CF3B32F008F91FF /* DumpRenderTreeWindow.mm */; };
 		A8D79CEB0FC28B2C004AC8FE /* DumpRenderTreeFileDraggingSource.m in Sources */ = {isa = PBXBuildFile; fileRef = A8D79CE90FC28B2C004AC8FE /* DumpRenderTreeFileDraggingSource.m */; };
@@ -315,7 +315,6 @@
 		53CBB830134E42F3001CE6A4 /* CyclicRedundancyCheck.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CyclicRedundancyCheck.cpp; sourceTree = "<group>"; };
 		53CBB831134E42F3001CE6A4 /* CyclicRedundancyCheck.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CyclicRedundancyCheck.h; sourceTree = "<group>"; };
 		5DE8AE4313A2C15800D6A37D /* libWebCoreTestSupport.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; path = libWebCoreTestSupport.dylib; sourceTree = BUILT_PRODUCTS_DIR; };
-		7CBBC3211DDFCF9A00786B9D /* TestOptions.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = TestOptions.mm; sourceTree = "<group>"; };
 		7CBBC3221DDFCF9A00786B9D /* TestOptions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TestOptions.h; sourceTree = "<group>"; };
 		80045AEB147718E7008290A8 /* AccessibilityNotificationHandler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = AccessibilityNotificationHandler.h; path = mac/AccessibilityNotificationHandler.h; sourceTree = "<group>"; };
 		80045AEC147718E7008290A8 /* AccessibilityNotificationHandler.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = AccessibilityNotificationHandler.mm; path = mac/AccessibilityNotificationHandler.mm; sourceTree = "<group>"; };
@@ -336,6 +335,7 @@
 		A16CD20D18907A7D0092363E /* BaseTarget.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = BaseTarget.xcconfig; path = mac/Configurations/BaseTarget.xcconfig; sourceTree = "<group>"; };
 		A19317FC1892855F001C52B1 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; name = Info.plist; path = ios/Info.plist; sourceTree = "<group>"; };
 		A19317FD18928738001C52B1 /* DumpRenderTreeApp.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = DumpRenderTreeApp.xcconfig; path = mac/Configurations/DumpRenderTreeApp.xcconfig; sourceTree = "<group>"; };
+		A30A21F72051D8960008FF42 /* TestOptions.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TestOptions.cpp; sourceTree = "<group>"; };
 		A803FF7409CAAD08009B2A37 /* DumpRenderTree.h */ = {isa = PBXFileReference; explicitFileType = sourcecode.cpp.h; fileEncoding = 4; path = DumpRenderTree.h; sourceTree = "<group>"; };
 		A817090308B164D300CCB9FB /* JavaScriptCore.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; path = JavaScriptCore.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		A84F608908B136DA00E9745F /* Cocoa.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Cocoa.framework; path = /System/Library/Frameworks/Cocoa.framework; sourceTree = "<absolute>"; };
@@ -472,7 +472,7 @@
 				A803FF7409CAAD08009B2A37 /* DumpRenderTree.h */,
 				BCA18C460C9B5B9400114369 /* DumpRenderTree.mm */,
 				A134E52F188FC27000901D06 /* DumpRenderTreeMain.mm */,
-				7CBBC3211DDFCF9A00786B9D /* TestOptions.mm */,
+				A30A21F72051D8960008FF42 /* TestOptions.cpp */,
 				7CBBC3221DDFCF9A00786B9D /* TestOptions.h */,
 				BC4741290D038A4C0072B006 /* JavaScriptThreading.h */,
 				BCA18C0A0C9B59EF00114369 /* DumpRenderTreeMac.h */,
@@ -1105,7 +1105,7 @@
 				BCB284D60CFA83D1007E533E /* PixelDumpSupportMac.mm in Sources */,
 				BCA18B660C9B08C200114369 /* PolicyDelegate.mm in Sources */,
 				BCA18B680C9B08C200114369 /* ResourceLoadDelegate.mm in Sources */,
-				7CBBC3231DDFCF9A00786B9D /* TestOptions.mm in Sources */,
+				A30A21F82051D8C40008FF42 /* TestOptions.cpp in Sources */,
 				BC0131DA0C9772010087317D /* TestRunner.cpp in Sources */,
 				BCA18B240C9B014B00114369 /* TestRunnerMac.mm in Sources */,
 				F4D423611DD5048200678290 /* TextInputControllerIOS.m in Sources */,

Tools/DumpRenderTree/PlatformMac.cmake

@@ -68,8 +68,6 @@ list(APPEND DumpRenderTree_Cpp_SOURCES
 )
 
 list(APPEND DumpRenderTree_ObjCpp_SOURCES
-    TestOptions.mm
-
     mac/AccessibilityCommonMac.mm
     mac/AccessibilityControllerMac.mm
     mac/AccessibilityNotificationHandler.mm

Tools/DumpRenderTree/TestOptions.mm → Tools/DumpRenderTree/TestOptions.cpp

@@ -26,9 +26,7 @@
 #include "config.h"
 #include "TestOptions.h"
 
-#include <Foundation/Foundation.h>
 #include <fstream>
-#include <string>
 
 static bool parseBooleanTestHeaderValue(const std::string& value)
 {
@@ -37,18 +35,15 @@ static bool parseBooleanTestHeaderValue(const std::string& value)
     if (value == "false")
         return false;
 
-    NSLog(@"Found unexpected value '%s' for boolean option. Expected 'true' or 'false'.", value.c_str());
+    LOG_ERROR("Found unexpected value '%s' for boolean option. Expected 'true' or 'false'.", value.c_str());
     return false;
 }
 
-TestOptions::TestOptions(NSURL *testURL, const TestCommand& command)
+TestOptions::TestOptions(const std::string& pathOrURL, const std::string& absolutePath)
 {
-    std::string path = command.absolutePath;
-    if (path.empty()) {
-        path = [testURL isFileURL] ? [testURL fileSystemRepresentation] : command.pathOrURL;
-        if (path.empty())
-            return;
-    }
+    const auto& path = absolutePath.empty() ? pathOrURL : absolutePath;
+    if (path.empty())
+        return;
 
     std::string options;
     std::ifstream testFile(path.data());
@@ -62,7 +57,7 @@ static bool parseBooleanTestHeaderValue(const std::string& value)
         return;
     size_t endLocation = options.find(endString, beginLocation);
     if (endLocation == std::string::npos) {
-        NSLog(@"Could not find end of test header in %s", path.c_str());
+        LOG_ERROR("Could not find end of test header in %s", path.c_str());
         return;
     }
     std::string pairString = options.substr(beginLocation + beginString.size(), endLocation - (beginLocation + beginString.size()));
@@ -73,39 +68,39 @@ static bool parseBooleanTestHeaderValue(const std::string& value)
             pairEnd = pairString.size();
         size_t equalsLocation = pairString.find("=", pairStart);
         if (equalsLocation == std::string::npos) {
-            NSLog(@"Malformed option in test header (could not find '=' character) in %s", path.c_str());
+            LOG_ERROR("Malformed option in test header (could not find '=' character) in %s", path.c_str());
             break;
         }
         auto key = pairString.substr(pairStart, equalsLocation - pairStart);
         auto value = pairString.substr(equalsLocation + 1, pairEnd - (equalsLocation + 1));
         if (key == "enableAttachmentElement")
-            this->enableAttachmentElement = parseBooleanTestHeaderValue(value);
+            enableAttachmentElement = parseBooleanTestHeaderValue(value);
         if (key == "useAcceleratedDrawing")
-            this->useAcceleratedDrawing = parseBooleanTestHeaderValue(value);
+            useAcceleratedDrawing = parseBooleanTestHeaderValue(value);
         else if (key == "enableIntersectionObserver")
-            this->enableIntersectionObserver = parseBooleanTestHeaderValue(value);
+            enableIntersectionObserver = parseBooleanTestHeaderValue(value);
         else if (key == "enableMenuItemElement")
-            this->enableMenuItemElement = parseBooleanTestHeaderValue(value);
+            enableMenuItemElement = parseBooleanTestHeaderValue(value);
         else if (key == "enableModernMediaControls")
-            this->enableModernMediaControls = parseBooleanTestHeaderValue(value);
+            enableModernMediaControls = parseBooleanTestHeaderValue(value);
         else if (key == "enablePointerLock")
-            this->enablePointerLock = parseBooleanTestHeaderValue(value);
+            enablePointerLock = parseBooleanTestHeaderValue(value);
         else if (key == "enableWebAuthentication")
-            this->enableWebAuthentication = parseBooleanTestHeaderValue(value);
+            enableWebAuthentication = parseBooleanTestHeaderValue(value);
         else if (key == "enableDragDestinationActionLoad")
-            this->enableDragDestinationActionLoad = parseBooleanTestHeaderValue(value);
+            enableDragDestinationActionLoad = parseBooleanTestHeaderValue(value);
         else if (key == "layerBackedWebView")
-            this->layerBackedWebView = parseBooleanTestHeaderValue(value);
+            layerBackedWebView = parseBooleanTestHeaderValue(value);
         else if (key == "enableIsSecureContextAttribute")
-            this->enableIsSecureContextAttribute = parseBooleanTestHeaderValue(value);
+            enableIsSecureContextAttribute = parseBooleanTestHeaderValue(value);
         else if (key == "enableInspectorAdditions")
-            this->enableInspectorAdditions = parseBooleanTestHeaderValue(value);
+            enableInspectorAdditions = parseBooleanTestHeaderValue(value);
         else if (key == "dumpJSConsoleLogInStdErr")
-            this->dumpJSConsoleLogInStdErr = parseBooleanTestHeaderValue(value);
+            dumpJSConsoleLogInStdErr = parseBooleanTestHeaderValue(value);
         else if (key == "allowCrossOriginSubresourcesToAskForCredentials")
-            this->allowCrossOriginSubresourcesToAskForCredentials = parseBooleanTestHeaderValue(value);
+            allowCrossOriginSubresourcesToAskForCredentials = parseBooleanTestHeaderValue(value);
         else if (key == "enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations")
-            this->enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations = parseBooleanTestHeaderValue(value);
+            enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations = parseBooleanTestHeaderValue(value);
         pairStart = pairEnd + 1;
     }
 }

Tools/DumpRenderTree/TestOptions.h

@@ -24,9 +24,8 @@
  */
 
 #pragma once
-#include "DumpRenderTree.h"
 
-@class NSURL;
+#include <string>
 
 struct TestOptions {
     bool enableAttachmentElement { false };
@@ -44,6 +43,6 @@ struct TestOptions {
     bool dumpJSConsoleLogInStdErr { false };
     bool allowCrossOriginSubresourcesToAskForCredentials { false };
 
-    TestOptions(NSURL*, const TestCommand&);
+    TestOptions(const std::string& pathOrURL, const std::string& absolutePath);
     bool webViewIsCompatibleWithOptions(const TestOptions&) const;
 };

Tools/DumpRenderTree/mac/DumpRenderTree.mm

@@ -1907,7 +1907,8 @@ static void runTest(const string& inputLine)
     NSString *informationString = [@"CRASHING TEST: " stringByAppendingString:testPath];
     WebKit::setCrashReportApplicationSpecificInformation((CFStringRef)informationString);
 
-    TestOptions options(url, command);
+    TestOptions options { [url isFileURL] ? [url fileSystemRepresentation] : pathOrURL, command.absolutePath };
+
     if (!mainFrameTestOptions || !options.webViewIsCompatibleWithOptions(mainFrameTestOptions.value())) {
         if (mainFrame)
             destroyWebViewAndOffscreenWindow([mainFrame webView]);

Tools/DumpRenderTree/win/DumpRenderTree.cpp

@@ -37,6 +37,7 @@
 #include "PixelDumpSupport.h"
 #include "PolicyDelegate.h"
 #include "ResourceLoadDelegate.h"
+#include "TestOptions.h"
 #include "TestRunner.h"
 #include "UIDelegate.h"
 #include "WebCoreTestSupport.h"
@@ -787,7 +788,6 @@ static void enableExperimentalFeatures(IWebPreferences* preferences)
     prefsPrivate->setWebAnimationsEnabled(TRUE);
     // FIXME: WebGL2
     // FIXME: WebRTC
-    prefsPrivate->setIsSecureContextAttributeEnabled(TRUE);
 }
 
 static void resetWebPreferencesToConsistentValues(IWebPreferences* preferences)
@@ -881,16 +881,23 @@ static void resetWebPreferencesToConsistentValues(IWebPreferences* preferences)
     prefsPrivate->setFetchAPIEnabled(TRUE);
     prefsPrivate->setShadowDOMEnabled(TRUE);
     prefsPrivate->setCustomElementsEnabled(TRUE);
-    prefsPrivate->setModernMediaControlsEnabled(FALSE);
     prefsPrivate->setResourceTimingEnabled(TRUE);
     prefsPrivate->setUserTimingEnabled(TRUE);
     prefsPrivate->setDataTransferItemsEnabled(TRUE);
-    prefsPrivate->setInspectorAdditionsEnabled(TRUE);
     prefsPrivate->clearNetworkLoaderSession();
 
     setAlwaysAcceptCookies(false);
 }
 
+static void setWebPreferencesForTestOptions(IWebPreferences* preferences, const TestOptions& options)
+{
+    COMPtr<IWebPreferencesPrivate6> prefsPrivate { Query, preferences };
+
+    prefsPrivate->setModernMediaControlsEnabled(options.enableModernMediaControls);
+    prefsPrivate->setIsSecureContextAttributeEnabled(options.enableIsSecureContextAttribute);
+    prefsPrivate->setInspectorAdditionsEnabled(options.enableInspectorAdditions);
+}
+
 static String applicationId()
 {
     DWORD processId = ::GetCurrentProcessId();
@@ -937,7 +944,7 @@ static void setDefaultsToConsistentValuesForTesting()
 #endif
 }
 
-static void resetWebViewToConsistentStateBeforeTesting()
+static void resetWebViewToConsistentStateBeforeTesting(const TestOptions& options)
 {
     COMPtr<IWebView> webView;
     if (FAILED(frame->webView(&webView))) 
@@ -983,8 +990,10 @@ static void resetWebViewToConsistentStateBeforeTesting()
     }
 
     COMPtr<IWebPreferences> preferences;
-    if (SUCCEEDED(webView->preferences(&preferences)))
+    if (SUCCEEDED(webView->preferences(&preferences))) {
         resetWebPreferencesToConsistentValues(preferences.get());
+        setWebPreferencesForTestOptions(preferences.get(), options);
+    }
 
     TestRunner::setSerializeHTTPLoads(false);
 
@@ -1155,7 +1164,9 @@ static void runTest(const string& inputLine)
 
     CFRelease(url);
 
-    resetWebViewToConsistentStateBeforeTesting();
+    TestOptions options { command.pathOrURL, command.absolutePath };
+
+    resetWebViewToConsistentStateBeforeTesting(options);
 
     ::gTestRunner = TestRunner::create(testURL.data(), command.expectedPixelHash);
     ::gTestRunner->setCustomTimeout(command.timeout);
@@ -1261,7 +1272,7 @@ static void runTest(const string& inputLine)
         }
     }
 
-    resetWebViewToConsistentStateBeforeTesting();
+    resetWebViewToConsistentStateBeforeTesting(options);
 
     // Loading an empty request synchronously replaces the document with a blank one, which is necessary
     // to stop timers, WebSockets and other activity that could otherwise spill output into next test's results.