NEW 126272
Refactor ViewState handling for drawing area / plugins
https://bugs.webkit.org/show_bug.cgi?id=126272
Summary Refactor ViewState handling for drawing area / plugins
Gavin Barraclough
Reported 2013-12-28 12:40:11 PST
Instead of all ViewState changes being handled by the WebPage, notify the DrawingArea & PluginView to better encapsulate.
Attachments
Fix (12.25 KB, patch)
2013-12-28 12:50 PST, Gavin Barraclough
buildbot: commit-queue-
Fix (12.53 KB, patch)
2013-12-28 13:28 PST, Gavin Barraclough
thorton: review+
Gavin Barraclough
Comment 1 2013-12-28 12:50:59 PST
Build Bot
Comment 2 2013-12-28 13:22:28 PST
Gavin Barraclough
Comment 3 2013-12-28 13:28:03 PST
Oliver Hunt
Comment 4 2013-12-28 19:21:36 PST
Comment on attachment 220079 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=220079&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2100 > + for (auto* pluginView : m_pluginViews) > + pluginView->viewStateDidChange(changed); randomly, could viewStateDidChange execute JS? (e.g. via an event handler?)
Gavin Barraclough
Comment 5 2013-12-28 22:51:43 PST
(In reply to comment #4) > (From update of attachment 220079 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220079&action=review > > > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2100 > > + for (auto* pluginView : m_pluginViews) > > + pluginView->viewStateDidChange(changed); > > randomly, could viewStateDidChange execute JS? (e.g. via an event handler?) Are you thinking WebPage::viewStateDidChange, or PluginView::viewStateDidChange specifically? WebPage::viewStateDidChange can cause JS execution (if the IsVisible state changes this may fire a DOM event, plus focus/blur events may fire?) – but PluginView::viewStateDidChange should just result in a couple of async messages being fired off to the plugin process.
Tim Horton
Comment 6 2014-01-02 11:42:14 PST
Comment on attachment 220079 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=220079&action=review > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:498 > + m_parameters.layerHostingMode = m_webPage->layerHostingMode(); does this need to be inside a HAVE(LAYER_HOSTING_IN_WINDOW_SERVER)?
Gavin Barraclough
Comment 7 2014-01-02 14:16:40 PST
Fixed in r161226
Note You need to log in before you can comment on or make changes to this bug.