Preparing for deferred image decoding for Chromium port. ImageSource is used to provide deferred image decoder for BitmapImage. However WebImageSkia also uses it Chromium WebKit API. WebImageSkia should use ImageDecoder directly.
Created attachment 162887 [details] Patch
Review ping.
Comment on attachment 162887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162887&action=review > Source/WebKit/chromium/src/WebImageSkia.cpp:55 > + OwnPtr<ImageDecoder> decoder(adoptPtr(ImageDecoder::create(*buffer.get(), ImageSource::AlphaPremultiplied, ImageSource::GammaAndColorProfileApplied))); ImageDecoder::create should call adoptPtr itself and return a PassOwnPtr. You don't need to fix that in this patch, but I just wanted to mention it. (Also, we tend to prefer the assignment form of the constructor for readability. > Source/WebKit/chromium/src/WebImageSkia.cpp:87 > + OwnPtr<NativeImageSkia> image = adoptPtr(frame->asNewNativeImage()); asNewNativeImage should probably also return a PassOwnPtr.
(In reply to comment #3) > (From update of attachment 162887 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=162887&action=review > > > Source/WebKit/chromium/src/WebImageSkia.cpp:55 > > + OwnPtr<ImageDecoder> decoder(adoptPtr(ImageDecoder::create(*buffer.get(), ImageSource::AlphaPremultiplied, ImageSource::GammaAndColorProfileApplied))); > > ImageDecoder::create should call adoptPtr itself and return a PassOwnPtr. You don't need to fix that in this patch, but I just wanted to mention it. (Also, we tend to prefer the assignment form of the constructor for readability. > > > Source/WebKit/chromium/src/WebImageSkia.cpp:87 > > + OwnPtr<NativeImageSkia> image = adoptPtr(frame->asNewNativeImage()); > > asNewNativeImage should probably also return a PassOwnPtr. Those are all platform specific implementations. Particularly ImageDecoder::create() on CG is not open-sourced (I can't find an implementation of it on CG). I'll file a bug to track this.
Comment on attachment 162887 [details] Patch Rejecting attachment 162887 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: it/chromium/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/chromium/WebKit.gypi Hunk #1 FAILED at 104. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/chromium/WebKit.gypi.rej patching file Source/WebKit/chromium/src/WebImageSkia.cpp patching file Source/WebKit/chromium/tests/WebImageTest.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13896303
Created attachment 164613 [details] Patch for landing
Comment on attachment 164613 [details] Patch for landing Rejecting attachment 164613 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: (s). patching file Source/WebKit/chromium/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/chromium/WebKit.gypi Hunk #1 FAILED at 101. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/chromium/WebKit.gypi.rej patching file Source/WebKit/chromium/src/WebImageSkia.cpp patching file Source/WebKit/chromium/tests/WebImageTest.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13903245
Created attachment 164614 [details] Patch for landing
Comment on attachment 164614 [details] Patch for landing Rejecting attachment 164614 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: (s). patching file Source/WebKit/chromium/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/chromium/WebKit.gypi Hunk #1 FAILED at 101. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/chromium/WebKit.gypi.rej patching file Source/WebKit/chromium/src/WebImageSkia.cpp patching file Source/WebKit/chromium/tests/WebImageTest.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13900292
Created attachment 164615 [details] Patch for landing
Comment on attachment 164615 [details] Patch for landing Clearing flags on attachment: 164615 Committed r128939: <http://trac.webkit.org/changeset/128939>
All reviewed patches have been landed. Closing bug.
Reverted r128939 for reason: Failing test_shell_tests Committed r128942: <http://trac.webkit.org/changeset/128942>
Created attachment 164637 [details] Patch
Created attachment 164638 [details] Patch
Please have another look. Patch was reverted because of a crash and bug is fixed in latest patch. I also added a test case to catch the bug.
> Particularly ImageDecoder::create() on CG is not open-sourced (I can't find an implementation of it on CG). I'll file a bug to track this. Ports that use CG don't use ImageDecoder::create. Thanks for filing the bugs about changing these functions.
Created attachment 164759 [details] Patch for landing
Comment on attachment 164759 [details] Patch for landing Clearing flags on attachment: 164759 Committed r129032: <http://trac.webkit.org/changeset/129032>