On Thu, 5 Dec 2019 05:42:10 GMT, Arun Joseph <ajos...@openjdk.org> wrote:
> On Thu, 28 Nov 2019 13:16:19 GMT, Arun Joseph <ajos...@openjdk.org> wrote: > >> On Wed, 20 Nov 2019 15:04:07 GMT, Kevin Rushforth <k...@openjdk.org> wrote: >> >>> On Wed, 20 Nov 2019 07:05:40 GMT, Arun Joseph <ajos...@openjdk.org> wrote: >>> >>>> Issue: Native part of WebView throws a DOMException and then, continues >>>> executing the rest of the function assuming that value is present. This >>>> causes the JVM to crash when retrieving the value. >>>> >>>> Fix: Return from the function if exception was raised (code is similar to >>>> exception handling in >>>> [WebKitLegacy/java/DOM/JavaTreeWalker.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/WebKitLegacy/java/DOM/JavaTreeWalker.cpp)) >>>> >>>> This fix also needs to be applied to all function calls in >>>> [WebKitLegacy/java/DOM](https://github.com/openjdk/jfx/tree/master/modules/javafx.web/src/main/native/Source/WebKitLegacy/java/DOM) >>>> functions which raises DOMError similar to createAttributeImpl(). >>>> >>>> ---------------- >>>> >>>> Commits: >>>> - acc52780: 8233747: JVM crash in >>>> com.sun.webkit.dom.DocumentImpl.createAttribute >>>> >>>> Changes: https://git.openjdk.java.net/jfx/pull/47/files >>>> Webrev: https://webrevs.openjdk.java.net/jfx/47/webrev.00 >>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8233747 >>>> Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod >>>> Patch: https://git.openjdk.java.net/jfx/pull/47.diff >>>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/47/head:pull/47 >>> >>> The proposed fix seems more like a workaround to me. There are dozens of >>> very similar calls to `raiseOnDOMError` in this and other files, so I would >>> think a more general solution is needed. >> >> For calls to `raiseOnDOMError()` with argument of type >> `ExceptionOr<Ref<T>>`, the returned value is again passed through >> `WTF::getPtr()`. This doesn't modify the value returned, but removing it >> will require changing about 40 function calls. > > In `createAttributeImpl` function in > [JavaDocument.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/WebKitLegacy/java/DOM/JavaDocument.cpp), > the value returned from `raiseOnDOMError` is passed to `WTF::getPtr` > function. Now there are two calls to `WTF::getPtr`, one from > `raiseOnDOMError` in the return statement and another in > `createAttributeImpl`, instead of just one. So, this approach has an > unnecessary call to `WTF::getPtr` which, if needed, has to removed at the > `createAttributeImpl` end. This doesn't modify the `Attr` object returned > because `WTF::getPtr` returns the same pointer when called multiple times. Thanks for the explanation. It seems fine the way it is, but if you want to file a follow-up issue, that would be fine, too. PR: https://git.openjdk.java.net/jfx/pull/47
