The plugins may dynamically switch between 2D and 3D rendering. Plugins need a way to inform the browser of such changes such that accelerated compositing can be switched on or off. More information in the chromium bug: http://code.google.com/p/chromium/issues/detail?id=72784
Created attachment 83854 [details] Adds a hook to WebPluginContainer to mark the containing element that it needs style recalc.
Comment on attachment 83854 [details] Adds a hook to WebPluginContainer to mark the containing element that it needs style recalc. View in context: https://bugs.webkit.org/attachment.cgi?id=83854&action=review > WebKit/chromium/public/WebPluginContainer.h:57 > + virtual void setNeedsStyleRecalc() = 0; It is really unfortunate to have to expose such a low-level detail to the embedder. Can you explain why you need this? Also, if we really need to expose this, then it seems like it should be exposed on WebElement as that is the wrapper for WebCore::Element methods.
I don't think the method should be named as such. Fundamentally, the WebPluginContainer asks the WebPlugin for its backing texture ID for compositing when it makes layout decisions. What needs to be exposed is a way for the WebPlugin to tell the WebPluginContainer that it has changed. The notion of style is internal to WebKit. I would call it something like "changedBackingTextureID". Or maybe better yet, change the API to remove WebPlugin::getBackingTextureID and add WebPluginContainer::setBackingTextureID which would do the proper invalidation. Thoughts ?
PPAPI plugins may dynamically switch between 2D (non-accelerated) and 3D rendering (accelerated). The only way to switch on/off accelerated compositing is to trigger a recalcStyle on the containing element, which in turn reevaluates whether accelerated compositing is required or not. WebPluginContainer::setNeedsStyleRecalc() seems similar to WebPluginContainer::invalidateRect(), which is used by the embedder to notify the containing element that something needs to be updated. May be there are better ways to do this. I am not too familiar with WebKit, so please suggest. I am fine with adding this function to WebElement, but it seemed like a const-wrapper to WebCore::Element. More details are provided in the chromium bug: http://code.google.com/p/chromium/issues/detail?id=72784
Comment on attachment 83854 [details] Adds a hook to WebPluginContainer to mark the containing element that it needs style recalc. View in context: https://bugs.webkit.org/attachment.cgi?id=83854&action=review >> WebKit/chromium/public/WebPluginContainer.h:57 >> + virtual void setNeedsStyleRecalc() = 0; > > It is really unfortunate to have to expose such a low-level detail to the embedder. > Can you explain why you need this? > > Also, if we really need to expose this, then it seems like it should be exposed > on WebElement as that is the wrapper for WebCore::Element methods. I don’t understand why a plug-in would have to ask for this explicitly. If the real issue is switching between 2D and 3D rendering, then the call should be about that. Not about “style recalc”, which is an internal detail of the web engine that could be subject to change. Specifically what is going to change about the plug-in? Presumably something that WebKit queries through the plug-in API, so what? That’s what the call should be named after.
Created attachment 84612 [details] Added WebPluginContainer::setBackingTextureId as suggested by Antoine I liked Antoine's suggestion of adding WebPluginContainer::setBackingTextureId(). I think it is much cleaner and at a higher level than my original solution. Please note that WebPlugin::getBackingTextureId() is now obsolete. I will remove it after this CL lands into chromium tree and I have made corresponding changes in chromium.
Attachment 84612 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/public/WebPluginContainer.h:61: The parameter name "id" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/src/WebPluginContainerImpl.h:87: The parameter name "id" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/chromium/src/WebPluginContainerImpl.cpp:321: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 3 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 84769 [details] Fixed style issues
Ping! I know Darin Fisher is on paternity leave. Who else can look at it?
Comment on attachment 84769 [details] Fixed style issues View in context: https://bugs.webkit.org/attachment.cgi?id=84769&action=review ok, but please fix the ChangeLog. > WebCore/ChangeLog:8 > + No new tests. (OOPS!) This is usually replaced with either a reference to new test, existing test that covers the change, or an explanation why the test is not necessary or possible.
Created attachment 85060 [details] Fixed ChangeLog
Comment on attachment 85060 [details] Fixed ChangeLog activate Iconian gateway!
Comment on attachment 85060 [details] Fixed ChangeLog Clearing flags on attachment: 85060 Committed r80584: <http://trac.webkit.org/changeset/80584>
All reviewed patches have been landed. Closing bug.