Summary: | WebView public APIs should have thread violation checks. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||||
Component: | WebKit API | Assignee: | Mark Lam <mark.lam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, darin, ggaren, sam, timothy | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Local Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 154204 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Mark Lam
2016-02-12 12:22:40 PST
Created attachment 271202 [details]
proposed patch.
Comment on attachment 271202 [details]
proposed patch.
r=me
Comment on attachment 271202 [details] proposed patch. Clearing flags on attachment: 271202 Committed r196527: <http://trac.webkit.org/changeset/196527> All reviewed patches have been landed. Closing bug. Comment on attachment 271202 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=271202&action=review > Source/WebKit/mac/WebView/WebView.mm:5846 > + WebCoreThreadViolationCheckRoundTwo(); This should have added a WebCoreThreadViolationCheckRoundThree and corresponding ThreadViolationRoundThree along with tagging the WebKit version these were added with a new WebKitLinkedOnOrAfter check in -[WebFrameView initWithFrame:]. Not doing this is a binary compatibility risk, since old app who might be calling these APIs on other threads will start getting exceptions, when they might have worked fine before. (In reply to comment #5) > Comment on attachment 271202 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271202&action=review > > > Source/WebKit/mac/WebView/WebView.mm:5846 > > + WebCoreThreadViolationCheckRoundTwo(); > > This should have added a WebCoreThreadViolationCheckRoundThree and > corresponding ThreadViolationRoundThree along with tagging the WebKit > version these were added with a new WebKitLinkedOnOrAfter check in > -[WebFrameView initWithFrame:]. Not doing this is a binary compatibility > risk, since old app who might be calling these APIs on other threads will > start getting exceptions, when they might have worked fine before. I'll roll out and redo as Round 3. Created attachment 271249 [details]
proposed patch 2: revised to use WebCoreThreadViolationCheckRoundThree().
Comment on attachment 271249 [details] proposed patch 2: revised to use WebCoreThreadViolationCheckRoundThree(). View in context: https://bugs.webkit.org/attachment.cgi?id=271249&action=review You are missing a call to setDefaultThreadViolationBehavior in -[WebFrameView initWithFrame:]. > Source/WebCore/platform/mac/ThreadCheck.mm:39 > +static ThreadViolationBehavior threadViolationBehavior[MaximumThreadViolationRound] = > + { RaiseExceptionOnThreadViolation, RaiseExceptionOnThreadViolation, RaiseExceptionOnThreadViolation }; I would not have put this on a newline. Created attachment 271253 [details]
proposed patch 3: address Tim's feedback.
Comment on attachment 271253 [details] proposed patch 3: address Tim's feedback. Clearing flags on attachment: 271253 Committed r196551: <http://trac.webkit.org/changeset/196551> All reviewed patches have been landed. Closing bug. |