JDK-8160757 : Implement overridePreference() for DRT framework
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2016-07-04
  • Updated: 2017-06-08
  • Resolved: 2016-07-05
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u112Fixed 9 masterFixed
Description
overridePreference() is not implemented for JavaFX.
Some DRT test cases test the behavior  by overriding the default preferences.
Like : 
fast/regions/css-regions-disabled.html
Comments
+1
05-07-2016
lgtm +1
05-07-2016
Please review, incorporated Arun's and Guru's review comments http://cr.openjdk.java.net/~asrivastava/8160757/webrev.02/
05-07-2016
modules/web/src/main/java/com/sun/webkit/WebPage.java: + try { + twkOverridePreference(getPage(), key, value); + return; + } finally { Remove the return statement. modules/web/src/main/native/Source/WebCore/platform/java/WebPage.cpp: + Settings& settings = page->settings(); + String nativePropertyName = String(env, propertyName); + nativePropertyName.remove(0, 6); // Remove Webkit You can use substring method of WTFString instead of remove. String nativePropertyName = String(env, propertyName).substring(6); Also you are assuming it should property name should start with "WebKit", please add an ASSERT. ASSERT(String(env, propertyName).startsWith("WebKit")); Otherwise remove the comparison optimization, which doesn't have significant impact. + String nativePropertyValue(String(env, propertyValue)); String nativePropertyValue(env, propertyValue) should work.
05-07-2016
+1 with nit 1. combine 1405 + 1406 (modules/web/src/main/java/com/sun/webkit/WebPage.java) 1405 twkOverridePreference(getPage(), key, value); 1406 return; to return twkOverridePreference(getPage(), key, value); 2. add blank line after 1410 (modules/web/src/main/java/com/sun/webkit/WebPage.java) 1410 } 1411 public float getZoomFactor(boolean textOnly) { 3. TestRunnerJava.cpp @remove blank line @126 125 CheckAndClearException(env); 126 127 }
05-07-2016
Please review , incorporated Arun's review comments: http://cr.openjdk.java.net/~asrivastava/8160757/webrev.01/ Tested on Mac platform also.
05-07-2016
changeset: 11e57bfbdbfb user: asrivastava date: Tue Jul 05 17:42:32 2016 +0530 Reviewed-by: arajkumar, ghb, mbilla URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/11e57bfbdbfb
05-07-2016
modules/web/src/main/native/Source/WebCore/mapfile-vers: + Java_com_sun_webkit_WebPage_twkOverridePreference; mapfile-macosx also should have an entry for newly exported function. modules/web/src/main/java/com/sun/javafx/webkit/drt/DumpRenderTree.java: + drt.webPage.overridePreference(attribute, attributeValue); + drt.setWaiting(false); What is the use of calling drt.setWaiting(false).? + private static void overridePreference(String attribute, String attributeValue) { Change the param name to "key", "value". modules/web/src/main/native/Source/WebCore/platform/java/WebPage.cpp: + Settings& settings = page->settings(); + String str = String(env, propertyName); + str.remove(0, 6); // Remove Webkit String settingsName = String(env, propertyName).remove(0, 6); + String nativePropertyName("set"); + nativePropertyName.append(str); + String nativePropertyValue(String(env, propertyValue)); Why do you append "set" string? + } else { + return; + } + return; +} Remove all redundant return statement.
04-07-2016
Implemented "overridePreference()" Please review http://cr.openjdk.java.net/~asrivastava/8160757/webrev.00/ Created pipeline to override preferences from WebPage.cpp Tested fast/regions/css-regions-disabled.html getting passed on Win and Linux platform
04-07-2016