Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2324793002:
Support WebP images in the IOSImageDecoderImpl (Closed)
DescriptionSupport WebP images in the IOSImageDecoderImpl
WebP images are not supported natively on iOS.
This CL adds WebP images decoding to
IOSImageDecoderImpl using webp_decoder.
Most of the work had been done by
markusheintz@chromium.org and then I took over the CL.
BUG=625081
Committed: https://crrev.com/a3e8310cf8a391aaaebe458ba2441257e42738a6
Cr-Commit-Position: refs/heads/master@{#423829}
Patch Set 1 #
Total comments: 6
Patch Set 2 : noyau@ comments. #
Total comments: 2
Patch Set 3 : rebase. #
Total comments: 2
Patch Set 4 : noyau@ comments. #
Total comments: 16
Patch Set 5 : sdefresne@ comments. #
Total comments: 2
Patch Set 6 : rebase + sdefresne@ comments. #
Messages
Total messages: 44 (25 generated)
Description was changed from ========== Support WebP images in the IOSImageDecoderImpl. BUG=625081 ========== to ========== Support WebP images in the IOSImageDecoderImpl WebP images are not supported natively on iOS. This CL adds WebP images decoding to IOSImageDecoderImpl using webp_decoder. Most of the work had been done by markusheintz@chromium.org and then I took over the CL. BUG=625081 ==========
vitaliii@chromium.org changed reviewers: + noyau@chromium.org
Hi, could you have a look?
https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... File ios/chrome/browser/suggestions/ios_image_decoder_impl.h (right): https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... ios/chrome/browser/suggestions/ios_image_decoder_impl.h:35: base::scoped_nsobject<NSData> data); This private use scoped_nsobject has the unfortunate side effect of polluting the factory with objectiveC. It's kind of a bummer. Have you considered moving the class in the mm file and only provide a function here of the form image_fetcher::ImageDecoder createIOSImageDecoder(taskrunner); https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... File ios/chrome/browser/suggestions/ios_image_decoder_impl.mm (right): https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:115: UIImage* ui_image = [[UIImage imageWithData:image_data scale:1] retain]; Don't do the retain here https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:119: gfx::Image gfx_image(ui_image); Do it here instead.
Patchset #2 (id:20001) has been deleted
Hi, please have a look at my fix. https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... File ios/chrome/browser/suggestions/ios_image_decoder_impl.h (right): https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... ios/chrome/browser/suggestions/ios_image_decoder_impl.h:35: base::scoped_nsobject<NSData> data); On 2016/09/09 10:18:04, noyau wrote: > This private use scoped_nsobject has the unfortunate side effect of polluting > the factory with objectiveC. It's kind of a bummer. > > Have you considered moving the class in the mm file and only provide a function > here of the form > > image_fetcher::ImageDecoder createIOSImageDecoder(taskrunner); No, I have not, but that is a great idea (though quite unusual to me). Done. https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... File ios/chrome/browser/suggestions/ios_image_decoder_impl.mm (right): https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:115: UIImage* ui_image = [[UIImage imageWithData:image_data scale:1] retain]; On 2016/09/09 10:18:04, noyau wrote: > Don't do the retain here Done. https://codereview.chromium.org/2324793002/diff/1/ios/chrome/browser/suggesti... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:119: gfx::Image gfx_image(ui_image); On 2016/09/09 10:18:04, noyau wrote: > Do it here instead. Done.
lgtm https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm (right): https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm:53: class IOSImageDecoderImplTest : public PlatformTest { I believe this test is now completely platform independent, except for the creation of the ImageDecoder. Can these tests also be applied to the other implementation of the image decoder?
On 2016/09/09 15:20:02, noyau wrote: > lgtm > > https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... > File ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm (right): > > https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... > ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm:53: class > IOSImageDecoderImplTest : public PlatformTest { > I believe this test is now completely platform independent, except for the > creation of the ImageDecoder. Can these tests also be applied to the other > implementation of the image decoder? I am not sure that I know a good way to do this. Would it be ok to move the test core into functions (in some test_utils file), which take a pointer to ImageDecoder, and then use these functions in tests for all decoders? Or should I just copy them into test file for chrome/browser/search/suggestions/image_decoder_impl.h?
On 2016/09/28 12:47:27, vitaliii wrote: > On 2016/09/09 15:20:02, noyau wrote: > > lgtm > > > > > https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... > > File ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm > (right): > > > > > https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... > > ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm:53: class > > IOSImageDecoderImplTest : public PlatformTest { > > I believe this test is now completely platform independent, except for the > > creation of the ImageDecoder. Can these tests also be applied to the other > > implementation of the image decoder? > > I am not sure that I know a good way to do this. > Would it be ok to move the test core into functions (in some test_utils file), > which take a pointer to ImageDecoder, and then use these functions in tests for > all decoders? > Or should I just copy them into test file for > chrome/browser/search/suggestions/image_decoder_impl.h? Mmm, you're right, the dependencies on chrome/ makes this hard to share. The code would have to move into components, but the implementations can't move there. Maybe a define a parameterized test in component and INSTANTIATE_TEST_CASE_P with the right impl in ios/ and chrome/ ? It doesn't have to be in this CL.
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi noyau, Could you have another look? It looks like the current way of creating NTPSnippetsService is wrong and I am not sure how to proceed. https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm (right): https://codereview.chromium.org/2324793002/diff/40001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl_unittest.mm:53: class IOSImageDecoderImplTest : public PlatformTest { On 2016/09/09 15:20:02, noyau wrote: > I believe this test is now completely platform independent, except for the > creation of the ImageDecoder. Can these tests also be applied to the other > implementation of the image decoder? Acknowledged. https://codereview.chromium.org/2324793002/diff/60001/ios/chrome/browser/ntp_... File ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc (right): https://codereview.chromium.org/2324793002/diff/60001/ios/chrome/browser/ntp_... ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc:171: createIOSImageDecoder(nullptr), This will break something once the feature is turned on. Should I reuse existing task_runner (which means that decoding will happen in the same thread as database) or create a new thread?
https://codereview.chromium.org/2324793002/diff/60001/ios/chrome/browser/ntp_... File ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc (right): https://codereview.chromium.org/2324793002/diff/60001/ios/chrome/browser/ntp_... ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc:171: createIOSImageDecoder(nullptr), On 2016/09/29 10:10:39, vitaliii wrote: > This will break something once the feature is turned on. > Should I reuse existing task_runner (which means that decoding will happen in > the same thread as database) or create a new thread? You can probably use the blocking pool, but it's arguably cleaner to create a new ThreadPool to do those decoding.
vitaliii@chromium.org changed reviewers: + markusheintz@google.com
[+ markusheintz since he is back] Hi noyau, could you have a look at my usage of SequencedWorkerPool? I do not have much experience in this area. Thanks!
vitaliii@chromium.org changed reviewers: + markusheintz@chromium.org - markusheintz@google.com
sdefresne@chromium.org changed reviewers: + sdefresne@chromium.org
https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/ntp_... File ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/ntp_... ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc:159: scoped_refptr<base::SequencedWorkerPool> image_decoder_worker_pool( Creation of base::SequencePool directly is really uncommon in Chromium code (there's only a few occurrence outside of tests https://cs.chromium.org/search/?q=new%5C+base::SequencedWorkerPool+-file:.*te...). Instead, the recommended way is to use the blocking pool from WebThread (for ios) or BrowserThread (for the other platforms), just like is done a few lines above. So I would remove this local variable and instead pass "task_runner" to "createIOSImageDecoder". If you want the tasks to be independent, then ask the blocking pool for another task runner (by adding another invocation of web::WebThread::GetBlockingPool()->GetSequencedTaskRunnerWithShutdownBehavior(...)). https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl.h (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.h:17: // The IOSImageDecoderImpl class is in .mm file, so that it could be used in .cc style: Declaration comments describe use of the function (when it is non-obvious); comments at the definition of a function describe operation (https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...). I would recommend to instead say something like this (the fact that the class is private is an implementation detail): // Factory for iOS specific implementation of image_fetcher::ImageDecoder. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.h:19: std::unique_ptr<image_fetcher::ImageDecoder> createIOSImageDecoder( style: Regular functions have mixed case; "cheap" functions may use lower case with underscores (https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...). Rename this to CreateIOSImageDecoder instead. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl.mm (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:43: }; DISALLOW_COPY_AND_ASSIGN(WebpDecoderDelegate); https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:84: explicit IOSImageDecoderImpl(scoped_refptr<base::TaskRunner> task_runner); const scoped_refptr<base::TaskRunner>& task_runner https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:137: base::scoped_nsobject<NSData> image_data) { const base::scoped_nsobject<NSData>& image_data https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:157: return std::unique_ptr<image_fetcher::ImageDecoder>( Please use base::MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)). No need to use std::move for a scoped_refptr<T> either. #include "base/memory/ptr_util.h" return base::MakeUnique<IOSImageDecoderImpl>(task_runner);
Hi, could you have another look? https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/ntp_... File ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/ntp_... ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc:159: scoped_refptr<base::SequencedWorkerPool> image_decoder_worker_pool( On 2016/10/05 09:15:28, sdefresne wrote: > Creation of base::SequencePool directly is really uncommon in Chromium code > (there's only a few occurrence outside of tests > https://cs.chromium.org/search/?q=new%5C+base::SequencedWorkerPool+-file:.*te...). > > Instead, the recommended way is to use the blocking pool from WebThread (for > ios) or BrowserThread (for the other platforms), just like is done a few lines > above. So I would remove this local variable and instead pass "task_runner" to > "createIOSImageDecoder". If you want the tasks to be independent, then ask the > blocking pool for another task runner (by adding another invocation of > web::WebThread::GetBlockingPool()->GetSequencedTaskRunnerWithShutdownBehavior(...)). Done, thanks for detailed explanation! https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl.h (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.h:17: // The IOSImageDecoderImpl class is in .mm file, so that it could be used in .cc On 2016/10/05 09:15:28, sdefresne wrote: > style: Declaration comments describe use of the function (when it is > non-obvious); comments at the definition of a function describe operation > (https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...). > > I would recommend to instead say something like this (the fact that the class is > private is an implementation detail): > > // Factory for iOS specific implementation of image_fetcher::ImageDecoder. Done. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.h:19: std::unique_ptr<image_fetcher::ImageDecoder> createIOSImageDecoder( On 2016/10/05 09:15:28, sdefresne wrote: > style: Regular functions have mixed case; "cheap" functions may use lower case > with underscores > (https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...). > > Rename this to CreateIOSImageDecoder instead. Done. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl.mm (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:43: }; On 2016/10/05 09:15:28, sdefresne wrote: > DISALLOW_COPY_AND_ASSIGN(WebpDecoderDelegate); Done. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:84: explicit IOSImageDecoderImpl(scoped_refptr<base::TaskRunner> task_runner); On 2016/10/05 09:15:28, sdefresne wrote: > const scoped_refptr<base::TaskRunner>& task_runner Done. However, I was advised against |const &| and the argument was https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md... Is this different for iOS? Or is this because there is no |const &| in |CreateIOSImageDecoder| and so the argument above is applied there? https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:137: base::scoped_nsobject<NSData> image_data) { On 2016/10/05 09:15:28, sdefresne wrote: > const base::scoped_nsobject<NSData>& image_data Done. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:157: return std::unique_ptr<image_fetcher::ImageDecoder>( On 2016/10/05 09:15:28, sdefresne wrote: > Please use base::MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)). > No need to use std::move for a scoped_refptr<T> either. > > #include "base/memory/ptr_util.h" > > return base::MakeUnique<IOSImageDecoderImpl>(task_runner); Done.
lgtm Code looks good as is, but if you want to pass object by value and use std::move, I'll also accept it. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl.mm (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:84: explicit IOSImageDecoderImpl(scoped_refptr<base::TaskRunner> task_runner); On 2016/10/06 06:35:09, vitaliii wrote: > On 2016/10/05 09:15:28, sdefresne wrote: > > const scoped_refptr<base::TaskRunner>& task_runner > Done. > > However, I was advised against |const &| and the argument was > > https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md... > > Is this different for iOS? > Or is this because there is no |const &| in |CreateIOSImageDecoder| and so the > argument above is applied there? Since the implementation does not use std::move() [line 108], then it is better to pass the scoped_refptr<> per const reference. If you want to pass it by copy, then you should have used std::move() in the implementation. So, I think we should either use "const scoped_refptr<>&" in IOSImageDecoderImpl constructor and CreateIOSImageDecoder or neither. Given that CreateIOSImageDecoder does not use refefence, maybe we should not here either and instead do the following: IOSImageDecoderImpl::IOSImageDecoderImpl(scoped_refptr<base::TaskRunner> task_runner) : task_runner_(std::move(task_runner)), weak_factory_(this) {} std::unique_ptr<image_fetcher::ImageDecoder> createIOSImageDecoder( scoped_refptr<base::TaskRunner> task_runner) { return base::MakeUnique<image_fetcher::ImageDecoder>( IOSImageDecoderImpl(std::move(task_runner)); } Sorry for the churn, I was just trying to be consistent in use a const-reference in the file (I should instead have followed the recommendation you pointed to).
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #6 (id:120001) has been deleted
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Fix. No need to look. https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... File ios/chrome/browser/suggestions/ios_image_decoder_impl.mm (right): https://codereview.chromium.org/2324793002/diff/80001/ios/chrome/browser/sugg... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:84: explicit IOSImageDecoderImpl(scoped_refptr<base::TaskRunner> task_runner); On 2016/10/06 15:53:06, sdefresne wrote: > On 2016/10/06 06:35:09, vitaliii wrote: > > On 2016/10/05 09:15:28, sdefresne wrote: > > > const scoped_refptr<base::TaskRunner>& task_runner > > Done. > > > > However, I was advised against |const &| and the argument was > > > > > https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md... > > > > Is this different for iOS? > > Or is this because there is no |const &| in |CreateIOSImageDecoder| and so the > > argument above is applied there? > > Since the implementation does not use std::move() [line 108], then it is better > to pass the scoped_refptr<> per const reference. If you want to pass it by copy, > then you should have used std::move() in the implementation. > > So, I think we should either use "const scoped_refptr<>&" in IOSImageDecoderImpl > constructor and CreateIOSImageDecoder or neither. Given that > CreateIOSImageDecoder does not use refefence, maybe we should not here either > and instead do the following: > > IOSImageDecoderImpl::IOSImageDecoderImpl(scoped_refptr<base::TaskRunner> > task_runner) > : task_runner_(std::move(task_runner)), weak_factory_(this) {} > > std::unique_ptr<image_fetcher::ImageDecoder> createIOSImageDecoder( > scoped_refptr<base::TaskRunner> task_runner) { > return base::MakeUnique<image_fetcher::ImageDecoder>( > IOSImageDecoderImpl(std::move(task_runner)); > } > > Sorry for the churn, I was just trying to be consistent in use a const-reference > in the file (I should instead have followed the recommendation you pointed to). No worries :) That is a very good point about std::move in the implementation, which I missed previously. I choose your second approach (adding std::move to the implementation), which agrees with the link above. Thanks for the explanation! https://codereview.chromium.org/2324793002/diff/100001/ios/chrome/browser/ntp... File ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc (right): https://codereview.chromium.org/2324793002/diff/100001/ios/chrome/browser/ntp... ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc:172: СreateIOSImageDecoder(task_runner), This is a cyrillic C. https://codereview.chromium.org/2324793002/diff/100001/ios/chrome/browser/sug... File ios/chrome/browser/suggestions/ios_image_decoder_impl.mm (right): https://codereview.chromium.org/2324793002/diff/100001/ios/chrome/browser/sug... ios/chrome/browser/suggestions/ios_image_decoder_impl.mm:161: std::unique_ptr<image_fetcher::ImageDecoder> СreateIOSImageDecoder( This is a cyrillic C.
The CQ bit was checked by vitaliii@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from noyau@chromium.org Link to the patchset: https://codereview.chromium.org/2324793002/#ps140001 (title: "rebase + sdefresne@ comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Support WebP images in the IOSImageDecoderImpl WebP images are not supported natively on iOS. This CL adds WebP images decoding to IOSImageDecoderImpl using webp_decoder. Most of the work had been done by markusheintz@chromium.org and then I took over the CL. BUG=625081 ========== to ========== Support WebP images in the IOSImageDecoderImpl WebP images are not supported natively on iOS. This CL adds WebP images decoding to IOSImageDecoderImpl using webp_decoder. Most of the work had been done by markusheintz@chromium.org and then I took over the CL. BUG=625081 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Support WebP images in the IOSImageDecoderImpl WebP images are not supported natively on iOS. This CL adds WebP images decoding to IOSImageDecoderImpl using webp_decoder. Most of the work had been done by markusheintz@chromium.org and then I took over the CL. BUG=625081 ========== to ========== Support WebP images in the IOSImageDecoderImpl WebP images are not supported natively on iOS. This CL adds WebP images decoding to IOSImageDecoderImpl using webp_decoder. Most of the work had been done by markusheintz@chromium.org and then I took over the CL. BUG=625081 Committed: https://crrev.com/a3e8310cf8a391aaaebe458ba2441257e42738a6 Cr-Commit-Position: refs/heads/master@{#423829} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/a3e8310cf8a391aaaebe458ba2441257e42738a6 Cr-Commit-Position: refs/heads/master@{#423829} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
