Bug 113285

Summary: document.createTouch crashes when document has no frame.
Product: WebKit Reporter: Takashi Sakamoto <tasak>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: abarth, ahmad.saleem792, ap, benjamin, commit-queue, darin, esprehn+autocc, jkjiang, kangil.han, kevers, myid.shin, rniwa, tonikitoo, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
repro.html
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch darin: review+

Description Takashi Sakamoto 2013-03-26 02:23:12 PDT
Created attachment 195039 [details]
repro.html

reported by fuzzer:

Crash type	UNKNOWN
Crash address	0x000000000b00
Crash state	- crash stack -
WebCore::Touch::Touch
WebCore::Document::createTouch
WebCore::DocumentV8Internal::createTouchMethodCallback

    #0 0x551f47 in WebCore::Frame::pageZoomFactor() const third_party/WebKit/Source/WebCore/page/Frame.h:155
    #1 0xadd78a in WebCore::Touch::Touch(WebCore::Frame*, WebCore::EventTarget*, unsigned int, int, int, int, int, int, int, float, float) third_party/WebKit/Source/WebCore/dom/Touch.cpp:72
    #2 0x9cdc59 in WebCore::Touch::create(WebCore::Frame*, WebCore::EventTarget*, unsigned int, int, int, int, int, int, int, float, float) third_party/WebKit/Source/WebCore/dom/Touch.h:47
    #3 0x9cdbbf in WebCore::Document::createTouch(WebCore::DOMWindow*, WebCore::EventTarget*, int, int, int, int, int, int, int, float, float, int&) const third_party/WebKit/Source/WebCore/dom/Document.cpp:5609
    #4 0x2820762 in WebCore::DocumentV8Internal::createTouchMethod(v8::Arguments const&) out/Release/obj/gen/webcore/bindings/V8Document.cpp:2648
    #5 0xd56ca3 in v8::internal::MaybeObject* v8::internal::HandleApiCallHelper<false>(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) v8/src/builtins.cc:1327

https://cluster-fuzz.appspot.com/testcase?key=171689017
Comment 1 Takashi Sakamoto 2013-03-26 02:38:42 PDT
This bug is caused by change r122286 (https://bugs.webkit.org/show_bug.cgi?id=88807).

Source/WebCore/dom/Touch.cpp:
@@Touch::Touch(Frame* frame, EventTarget* target, unsigned identifier, int screenX
    , m_rotationAngle(rotationAngle)
    , m_force(force)
{
+   float scaleFactor = frame->pageZoomFactor() * frame->frameScaleFactor();
+   float x = pageX * scaleFactor;
+   float y = pageY * scaleFactor;
+   m_absoluteLocation = roundedLayoutPoint(FloatPoint(x, y));
}

Need to check whether frame is available or not.
Comment 2 Miyoung Shin 2014-04-12 18:46:53 PDT
Created attachment 229215 [details]
Patch
Comment 3 Antonio Gomes 2014-04-12 18:52:30 PDT
Comment on attachment 229215 [details]
Patch

LayoutTest?
Comment 4 Miyoung Shin 2014-04-12 19:20:43 PDT
(In reply to comment #3)
> (From update of attachment 229215 [details])
> LayoutTest?

Ok. I got it
Comment 5 Miyoung Shin 2014-04-13 02:03:27 PDT
Created attachment 229226 [details]
Patch
Comment 6 Benjamin Poulain 2014-04-14 13:33:28 PDT
Comment on attachment 229226 [details]
Patch

Please generalize the code from mouse events instead.
Comment 7 Brian Burg 2014-04-14 17:02:48 PDT
Comment on attachment 229226 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229226&action=review

> Source/WebCore/ChangeLog:9
> +        Because it is possible that the Document has no the Frame.

"Because it is..." isn't necessary, you can delete that line.

> LayoutTests/fast/events/touch/document-create-touch-new-document.html:15
> +<p id="description"></p>

You may want to state the situation being tested here (i.e., "Should not crash when creating touch event in document without a frame")
Comment 8 Miyoung Shin 2014-04-18 08:39:24 PDT
Created attachment 229646 [details]
Patch
Comment 9 WebKit Commit Bot 2014-04-18 08:40:09 PDT
Attachment 229646 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/MouseRelatedEvent.cpp:47:  Wrong number of spaces before statement. (expected: 41)  [whitespace/indent] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Miyoung Shin 2014-04-18 08:43:18 PDT
(In reply to comment #6)
> (From update of attachment 229226 [details])
> Please generalize the code from mouse events instead.

Hi, I've uploaded new patch.
There is the style checking error, but I'm not sure whether I should fix it or not because It's already existed.
I hope to get your feedback.
Thanks.
Comment 11 Miyoung Shin 2014-04-18 08:44:44 PDT
(In reply to comment #7)
> (From update of attachment 229226 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229226&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Because it is possible that the Document has no the Frame.
> 
> "Because it is..." isn't necessary, you can delete that line.
> 
> > LayoutTests/fast/events/touch/document-create-touch-new-document.html:15
> > +<p id="description"></p>
> 
> You may want to state the situation being tested here (i.e., "Should not crash when creating touch event in document without a frame")

==> I've uloaded new patch to change some test code regarding to your comment
Comment 12 Darin Adler 2014-04-19 14:37:48 PDT
Comment on attachment 229646 [details]
Patch

It is *not* good to store a Frame* in every event, which is what this patch does. This looks like overkill. These helper functions don’t need to be put into a class, and we really don’t want to get multiple inheritance and virtual functions in the event classes just to do a bit more code sharing.
Comment 13 Miyoung Shin 2014-04-21 14:30:10 PDT
Created attachment 229831 [details]
Patch
Comment 14 WebKit Commit Bot 2014-04-21 14:31:27 PDT
Attachment 229831 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/ComputeContent.h:31:  #ifndef header guard has wrong style, please use: ComputeContent_h  [build/header_guard] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Miyoung Shin 2014-04-21 14:38:59 PDT
Created attachment 229833 [details]
Patch
Comment 16 Ahmad Saleem 2022-06-21 09:48:39 PDT
Based on MDN - https://developer.mozilla.org/en-US/docs/Web/API/Document/createTouch

It is deprecated standard and is not supported by browsers (e.g. referring to compatibility table). Although, I am able to see some reference to "createTouch" in Webkit Github source code mirror in following files:

Source/WebCore/dom/DocumentTouch.h
Source/WebCore/dom/DocumentTouch.cpp

Further, I am not able to run "repo.html" test and it shows error in console but using "patch" and getting layout test out of it (Changed into below JSFiddle), it shows "PASSED".

Link - https://jsfiddle.net/sj9pc2m3/

Is something needed here or considering the test now pass from this patch reflects that it was fixed along the way? Thanks!
Comment 17 Radar WebKit Bug Importer 2022-06-21 09:49:06 PDT
<rdar://problem/95609522>
Comment 18 Ryosuke Niwa 2022-06-21 13:53:56 PDT
We don't crash with the attached test case anymore.