Bug 126272 - Refactor ViewState handling for drawing area / plugins
Summary: Refactor ViewState handling for drawing area / plugins
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-28 12:40 PST by Gavin Barraclough
Modified: 2014-01-02 14:16 PST (History)
3 users (show)

See Also:


Attachments
Fix (12.25 KB, patch)
2013-12-28 12:50 PST, Gavin Barraclough
buildbot: commit-queue-
Details | Formatted Diff | Diff
Fix (12.53 KB, patch)
2013-12-28 13:28 PST, Gavin Barraclough
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2013-12-28 12:40:11 PST
Instead of all ViewState changes being handled by the WebPage, notify the DrawingArea & PluginView to better encapsulate.
Comment 1 Gavin Barraclough 2013-12-28 12:50:59 PST
Created attachment 220077 [details]
Fix
Comment 2 Build Bot 2013-12-28 13:22:28 PST
Comment on attachment 220077 [details]
Fix

Attachment 220077 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6287263306612736
Comment 3 Gavin Barraclough 2013-12-28 13:28:03 PST
Created attachment 220079 [details]
Fix
Comment 4 Oliver Hunt 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?)
Comment 5 Gavin Barraclough 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.
Comment 6 Tim Horton 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)?
Comment 7 Gavin Barraclough 2014-01-02 14:16:40 PST
Fixed in r161226