Bug 159327

Summary: HTMLMediaElement::resume() may cause JavaScript execution
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed patch. none

Description Chris Dumez 2016-06-30 20:43:19 PDT 
HTMLMediaElement::resume() may cause JavaScriptExecution, which is not allowed and will cause crashes such as this one:

Thread 0 Crashed ↩:
0   WebCore                       	0x0000000195435fd8 WebCore::ScriptExecutionContext::didCreateActiveDOMObject(WebCore::ActiveDOMObject&) + 52 (ScriptExecutionContext.cpp:332)
1   WebCore                       	0x00000001955238ec WebCore::SuspendableTimer::SuspendableTimer(WebCore::ScriptExecutionContext&) + 36 (SuspendableTimer.cpp:35)
2   WebCore                       	0x0000000194b986b4 WebCore::DOMTimer::DOMTimer(WebCore::ScriptExecutionContext&, std::__1::unique_ptr<WebCore::ScheduledAction, std::__1::default_delete<WebCore::ScheduledAction> >, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000l> >, bool) + 64 (DOMTimer.cpp:172)
3   WebCore                       	0x0000000194b989b4 WebCore::DOMTimer::install(WebCore::ScriptExecutionContext&, std::__1::unique_ptr<WebCore::ScheduledAction, std::__1::default_delete<WebCore::ScheduledAction> >, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000l> >, bool) + 84 (DOMTimer.cpp:179)
4   WebCore                       	0x0000000194ba3c94 WebCore::DOMWindow::setTimeout(std::__1::unique_ptr<WebCore::ScheduledAction, std::__1::default_delete<WebCore::ScheduledAction> >, int, int&) + 52 (DOMWindow.cpp:1599)
5   WebCore                       	0x0000000194f46cec WebCore::JSDOMWindow::setTimeout(JSC::ExecState&) + 320 (JSDOMWindowCustom.cpp:576)
6   WebCore                       	0x0000000194f3f46c WebCore::jsDOMWindowInstanceFunctionSetTimeout(JSC::ExecState*) + 188 (JSDOMWindow.cpp:26371)
7   ???                           	0x000000012dd2c030 0 + 5063753776
8   JavaScriptCore                	0x000000019433358c llint_entry + 24748
9   JavaScriptCore                	0x000000019433358c llint_entry + 24748
10  JavaScriptCore                	0x000000019432d318 vmEntryToJavaScript + 264
11  JavaScriptCore                	0x00000001941f7a50 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 168 (JITCode.cpp:80)
12  JavaScriptCore                	0x0000000193c49f70 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 368 (Interpreter.cpp:1015)
13  JavaScriptCore                	0x00000001941614b8 JSC::callSetter(JSC::ExecState*, JSC::JSValue, JSC::JSValue, JSC::JSValue, JSC::ECMAMode) + 320 (GetterSetter.cpp:105)
14  JavaScriptCore                	0x00000001942870d4 JSC::JSObject::putInlineSlow(JSC::ExecState*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&) + 2608 (JSObject.cpp:552)
15  JavaScriptCore                	0x0000000193c44bbc JSC::JSObject::put(JSC::JSCell*, JSC::ExecState*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&) + 552 (JSObjectInlines.h:81)
16  WebCore                       	0x0000000194d39714 WebCore::HTMLMediaElement::setControllerJSProperty(char const*, JSC::JSValue) + 468 (HTMLMediaElement.cpp:6524)
17  WebCore                       	0x0000000194b237d0 WebCore::Document::pageScaleFactorChangedAndStable() + 96 (Document.cpp:4878)
18  WebCore                       	0x00000001952829c4 WebCore::Page::setPageScaleFactor(float, WebCore::IntPoint const&, bool) + 212 (Page.cpp:820)
19  WebKit                        	0x00000001992e7064 WebKit::WebPage::scalePage(double, WebCore::IntPoint const&) + 340 (WebPage.cpp:1548)
20  WebKit                        	0x00000001992f554c WebKit::WebPage::restorePageState(WebCore::HistoryItem const&) + 668 (WebPageIOS.mm:298)
21  WebCore                       	0x00000001947ecc1c WebCore::FrameLoader::didFirstLayout() + 76 (FrameLoader.cpp:2399)
22  WebCore                       	0x0000000194ca06b0 WebCore::FrameView::fireLayoutRelatedMilestonesIfNeeded() + 64 (FrameView.cpp:4800)
23  WebCore                       	0x00000001947ec494 WebCore::FrameView::performPostLayoutTasks() + 224 (FrameView.cpp:3175)
24  WebCore                       	0x00000001947e8680 WebCore::FrameView::layout(bool) + 3536 (FrameView.cpp:1493)
25  WebCore                       	0x000000019483fe60 WebCore::Document::updateLayout() + 260 (Document.cpp:1985)
26  WebCore                       	0x0000000194b1b0b0 WebCore::Document::updateLayoutIgnorePendingStylesheets(WebCore::Document::RunPostLayoutTasks) + 240 (Document.cpp:2017)
27  WebKit                        	0x000000019939fc38 WebKit::inlineVideoFrame(WebCore::HTMLVideoElement&) + 56 (WebVideoFullscreenManager.mm:58)
28  WebKit                        	0x000000019939f8b4 WebKit::WebVideoFullscreenManager::enterVideoFullscreenForVideoElement(WebCore::HTMLVideoElement&, unsigned int) + 216 (WebVideoFullscreenManager.mm:280)
29  WebCore                       	0x0000000194d37624 WebCore::HTMLMediaElement::enterFullscreen(unsigned int) + 176 (HTMLMediaElement.cpp:5394)
30  WebCore                       	0x0000000194d308fc WebCore::HTMLMediaElement::updatePlayState() + 396 (HTMLMediaElement.cpp:4862)
31  WebCore                       	0x0000000194d36f84 WebCore::HTMLMediaElement::resume() + 92 (HTMLMediaElement.cpp:4932)
32  WebCore                       	0x00000001948f1e6c WebCore::ScriptExecutionContext::resumeActiveDOMObjects(WebCore::ActiveDOMObject::ReasonForSuspension) + 136 (ScriptExecutionContext.cpp:271)
33  WebCore                       	0x0000000194b23244 WebCore::Document::resume(WebCore::ActiveDOMObject::ReasonForSuspension) + 216 (Document.cpp:2467)
34  WebCore                       	0x00000001948f1a98 WebCore::CachedFrameBase::restore() + 112 (CachedFrame.cpp:95)
35  WebCore                       	0x00000001948f1948 WebCore::FrameLoader::open(WebCore::CachedFrameBase&) + 744 (FrameLoader.cpp:2123)
36  WebCore                       	0x00000001949c76f0 WebCore::CachedPage::restore(WebCore::Page&) + 32 (CachedPage.cpp:77)
37  WebCore                       	0x00000001947d8a14 WebCore::FrameLoader::commitProvisionalLoad() + 756 (FrameLoader.cpp:1831)
Comment 1 Chris Dumez 2016-06-30 20:45:10 PDT 
Here is seems to be because of setPausedInternal(false) which calls updatePlayState(). We probably want to so this asynchronously instead.
Comment 2 Radar WebKit Bug Importer 2016-07-01 08:25:01 PDT 
<rdar://problem/27131641>
Comment 3 Eric Carlson 2016-07-01 12:30:46 PDT 
Created attachment 282568 [details]
Proposed patch.
Comment 4 WebKit Commit Bot 2016-07-01 12:33:26 PDT 
Attachment 282568 [details] did not pass style-queue:


ERROR: Source/WebCore/html/HTMLMediaElement.h:695:  The parameter name "updateState" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/html/HTMLMediaElement.h:775:  The parameter name "updateState" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Eric Carlson 2016-07-01 12:43:08 PDT 
(In reply to comment #4)
> Attachment 282568 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/html/HTMLMediaElement.h:695:  The parameter name
> "updateState" adds no information, so it should be removed. 
> [readability/parameter_name] [5]
> ERROR: Source/WebCore/html/HTMLMediaElement.h:775:  The parameter name
> "updateState" adds no information, so it should be removed. 
> [readability/parameter_name] [5]
> Total errors found: 2 in 4 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

https://bugs.webkit.org/show_bug.cgi?id=159362
Comment 6 Eric Carlson 2016-07-01 13:33:11 PDT 
Committed r202749: http://trac.webkit.org/changeset/202749