[GTK] Separate image encoding from dataURL construction
Created attachment 135575 [details] Patch
Comment on attachment 135575 [details] Patch This seems like a nice refactor, but is there some reason that it will be useful in the future?
http://dev.w3.org/html5/spec/the-canvas-element.html#dom-canvas-toblob
Comment on attachment 135575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135575&action=review > Source/WebCore/platform/graphics/gtk/ImageBufferGtk.cpp:70 > + base64Encode(reinterpret_cast<const char*>(buffer.get()), bufferSize, base64Data); It seems unusual that we need this reinpterpret_cast here. Perhaps you could try removing this before landing?
Happy to try removing the reinterpret_cast<> and try landing.
Created attachment 136382 [details] Patch for landing
Other questions: does the SupportedImageMIMETypeForEncoding registry for GTK contain items that don't begin with "image/"? I'd be surprised, and it seems these lines are redundant, due to the ASSERT at the start of ImageBuffer::toDataURL() ... http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/gtk/ImageBufferGtk.cpp#L39 39 if (!mimeType.startsWith("image/")) 40 return "data:,";
Oh, and is gdk_pixbuf_save_to_buffer() thread-safe? I don't see anything in the docs about that.
Comment on attachment 136382 [details] Patch for landing Clearing flags on attachment: 136382 Committed r113742: <http://trac.webkit.org/changeset/113742>
All reviewed patches have been landed. Closing bug.
(In reply to comment #7) > Other questions: does the SupportedImageMIMETypeForEncoding registry for GTK contain items that don't begin with "image/"? I'd be surprised, and it seems these lines are redundant, due to the ASSERT at the start of ImageBuffer::toDataURL() ... > > http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/gtk/ImageBufferGtk.cpp#L39 > > 39 if (!mimeType.startsWith("image/")) > 40 return "data:,"; Filed bug 83657 about that if you'd like to review.