We need to avoid plugins running script when being painted. Currently this can happen when Safari takes a snapshot, which involves a flattening paint. In that situation, we ask plugins to draw via a draw event, and they sometimes run script in this callback (or as a result of processing pending requests at this time).
<rdar://problem/8093653>
Created attachment 77269 [details] Patch
Comment on attachment 77269 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77269&action=review > WebCore/page/FrameView.cpp:2140 > + if (flatteningPaint && !document->ownerElement()) > + notifyWidgetsInAllFrames(BeforeFlatteningPaint); This says !ownerElement. > WebCore/page/FrameView.cpp:2153 > + if (flatteningPaint && document->ownerElement()) > + notifyWidgetsInAllFrames(AfterFlatteningPaint); This says ownerElement. One of them must be wrong!
Created attachment 77273 [details] Patch
Comment on attachment 77269 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77269&action=review > WebCore/platform/Widget.h:113 > +enum WidgetNotification { BeforeFlatteningPaint, AfterFlatteningPaint }; How about WillPaintFlattened and DidPaintFlattened? > WebKit/mac/Plugins/WebBaseNetscapePluginView.mm:584 > + NSImage* snapshot = [[NSImage alloc] initWithSize: [self bounds].size]; Misplaced * > WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1587 > + switch (notification) { > + case BeforeFlatteningPaint: { > + BEGIN_BLOCK_OBJC_EXCEPTIONS; > + [(WebBaseNetscapePluginView *)platformWidget() cacheSnapshot]; > + END_BLOCK_OBJC_EXCEPTIONS; > + break; > + } > + case AfterFlatteningPaint: { > + BEGIN_BLOCK_OBJC_EXCEPTIONS; > + [(WebBaseNetscapePluginView *)platformWidget() clearCachedSnapshot]; > + END_BLOCK_OBJC_EXCEPTIONS; > + break; > + } Current WebKit style is to give the case labels the same indentation as the switch statement.
Comment on attachment 77273 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77273&action=review This looks good. I didn’t spot any obvious problems. I do worry that the guarantees of no plug-in calls during the drawing process here are indirect. I’d prefer a design that made a firmer guarantee that we will draw without calling the plug-in. > WebCore/rendering/RenderView.cpp:569 > +size_t RenderView::getRetainedWidgets(Vector<RenderWidget*>& renderWidgets) Doesn’t seem all that useful to return the size. Why not just call size on the vector? > WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1576 > + switch (notification) { > + case BeforeFlatteningPaint: { WebKit coding style says to line the case up with the switch rather than indenting.
http://trac.webkit.org/changeset/74524