Allow the bundle to specify the images to use.
<rdar://problem/12813842>
Created attachment 177713 [details] Patch
Comment on attachment 177713 [details] Patch Attachment 177713 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15159206
Comment on attachment 177713 [details] Patch Attachment 177713 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15160179
Created attachment 177720 [details] Patch
Comment on attachment 177720 [details] Patch Attachment 177720 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15133796
Comment on attachment 177720 [details] Patch Attachment 177720 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15152360
Comment on attachment 177720 [details] Patch Attachment 177720 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/15152358
Comment on attachment 177720 [details] Patch Attachment 177720 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15148445
Comment on attachment 177720 [details] Patch Attachment 177720 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15152374
Comment on attachment 177720 [details] Patch Attachment 177720 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/15159276
Created attachment 177766 [details] Patch
Created attachment 178195 [details] Patch
Created attachment 178226 [details] Patch
Created attachment 178228 [details] Patch
Comment on attachment 178228 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178228&action=review > Source/WebCore/page/ChromeClient.h:376 > + virtual PassRefPtr<Image> generatePlugInStartButtonImage(RenderSnapshottedPlugIn::ButtonSize) const { return 0; } Why 'generate'? Why not just plugInStartButtonImage() ? > Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:138 > + if (initializedImages[arrayIndex]) > + return 0; Is this just to avoid multiple aborted attempts to get the image? Do we expect that ever to happen? > Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:154 > + Image* buttonImage = 0; > + if (plugInImageElement()->hovered()) > + buttonImage = startButtonImage(m_buttonType); This logic is a bit odd. Why not just early return if not hovered? > Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:245 > + m_buttonRect = tryToFitStartButton(ButtonSizeLarge, rect); > + if (!m_buttonRect.isEmpty()) { > + m_buttonType = ButtonSizeLarge; > + return; > + } > + > + m_buttonRect = tryToFitStartButton(ButtonSizeSmall, rect); > + if (!m_buttonRect.isEmpty()) { > + m_buttonType = ButtonSizeSmall; > + return; > + } > + > + m_buttonType = NoButton; I'm not sure why you have to run this logic at layout time. Can't you just choose what to paint at paint time?
Comment on attachment 178228 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178228&action=review >> Source/WebCore/page/ChromeClient.h:376 >> + virtual PassRefPtr<Image> generatePlugInStartButtonImage(RenderSnapshottedPlugIn::ButtonSize) const { return 0; } > > Why 'generate'? Why not just plugInStartButtonImage() ? Renamed. This was originally connected to the idea that via the WK2 API we have to copy the data to a bitmap image every time we make this call, but since the code already caches those images, this is less important. >> Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:138 >> + return 0; > > Is this just to avoid multiple aborted attempts to get the image? Do we expect that ever to happen? If a client decides not to provide an image, then we will already ask for it. So I wanted to avoid that. Clients that implement this probably will provide all buttons. >> Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:154 >> + buttonImage = startButtonImage(m_buttonType); > > This logic is a bit odd. Why not just early return if not hovered? Yes, I've updated the logic here. >> Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:245 >> + m_buttonType = NoButton; > > I'm not sure why you have to run this logic at layout time. Can't you just choose what to paint at paint time? I thought it might be less performant to do this on every paint, but I can move it. Moving it would also obviate the need for these instance variables.
Committed r137067: <http://trac.webkit.org/changeset/137067>