Bug 154059

Summary: WebFrame _stringByEvaluatingJavaScriptFromString:forceUserGesture: should assert that it is being called from the "main" thread.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: WebKit Misc.Assignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, beidson, commit-queue, darin, ggaren, manian, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. none

Description Mark Lam 2016-02-09 18:54:25 PST 
This makes it so that misbehaving clients which call it (indirectly) from another thread (not the main thread) will fail faster.  Otherwise, we get potential memory corruption that results in strange crashes elsewhere later.
Comment 1 Radar WebKit Bug Importer 2016-02-10 10:38:42 PST 
<rdar://problem/24590120>
Comment 2 Mark Lam 2016-02-10 10:57:12 PST 
Created attachment 271005 [details]
proposed patch.
Comment 3 Geoffrey Garen 2016-02-10 13:26:37 PST 
Comment on attachment 271005 [details]
proposed patch.

r=me
Comment 4 WebKit Commit Bot 2016-02-10 14:17:00 PST 
Comment on attachment 271005 [details]
proposed patch.

Clearing flags on attachment: 271005

Committed r196395: <http://trac.webkit.org/changeset/196395>
Comment 5 WebKit Commit Bot 2016-02-10 14:17:02 PST 
All reviewed patches have been landed.  Closing bug.
Comment 6 Darin Adler 2016-02-11 07:26:32 PST 
We typically use Objective-C exceptions rather than RELEASE_ASSERT for this kind of thread restriction. These are easier for developers to understand than a crash.
Comment 7 Darin Adler 2016-02-11 08:33:58 PST 
Our previous work on this uses these functions:

WebCoreThreadViolationCheckRoundOne
WebCoreThreadViolationCheckRoundTwo

from the header ThreadCheck.h
Comment 8 Darin Adler 2016-02-11 08:40:17 PST 
Instead, or possibly in addition to, this RELEASE_ASSERT, we should add WebCoreThreadViolationCheckRoundTwo to the public API entry point -[WebView stringByEvaluatingJavaScriptFromString:] or come up with a WebCoreThreadViolationCheckRoundThree. And possibly add it to the internal bottleneck method as well.

A lot of thought went into how WebCoreThreadViolationCheck works, and it’s designed for just this sort of situation.

On the other hand, it’s not surprising we have forgotten about this since that work was done 7 years ago!

Bug 22976 was when we created all of this.
Comment 9 Mark Lam 2016-02-12 16:15:44 PST 
(In reply to comment #8)
> A lot of thought went into how WebCoreThreadViolationCheck works, and it’s
> designed for just this sort of situation.

I've added thread violation checks to WebView's public APIs in https://bugs.webkit.org/show_bug.cgi?id=154183.