|
|
Created:
3 years, 10 months ago by reed1 Modified:
3 years, 10 months ago CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-api_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, dshwang, drott+blinkwatch_chromium.org, krit, feature-media-reviews_chromium.org, fmalita+watch_chromium.org, jam, jbroman, Justin Novosad, kinuko+watch, mlamouri+watch-content_chromium.org, pdr+graphicswatchlist_chromium.org, posciak+watch_chromium.org, rwlbuis, Stephen Chennney Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionswitch to new ImageGenerator API
Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator>
BUG=skia:5077
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
TBR=hubbe
Review-Url: https://codereview.chromium.org/2706453003
Cr-Commit-Position: refs/heads/master@{#451848}
Committed: https://chromium.googlesource.com/chromium/src/+/08cd16372dba031d8adac493288fbe1a8c67a678
Patch Set 1 #
Total comments: 4
Patch Set 2 : fix skia_common #Patch Set 3 : fix typo #Patch Set 4 : fix new site in tile_manager_unittest.cc #
Messages
Total messages: 57 (36 generated)
The CQ bit was checked by reed@google.com 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...
Description was changed from ========== switch to new ImageGenerator API BUG= ========== to ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG= ==========
reed@google.com changed reviewers: + fmalita@chromium.org
reed@google.com changed reviewers: + chrishtr@chromium.org, hubbe@chromium.org, jochen@chromium.org
need owners for WebKit/public, content, media for refactoring CL
lgtm https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp (right): https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp:331: std::unique_ptr<DecodingImageGenerator> generator(new DecodingImageGenerator( nit: WTF::makeUnique(...);
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/public/p... File third_party/WebKit/public/platform/WebImageGenerator.h (right): https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/public/p... third_party/WebKit/public/platform/WebImageGenerator.h:44: static std::unique_ptr<SkImageGenerator> create(sk_sp<SkData>); It's not clear to me immediately from digging into the Skia code what happens with the pointer that is returned from this. Does it cleanly obey unique_ptr semantics?
Description was changed from ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG= ========== to ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/public/p... File third_party/WebKit/public/platform/WebImageGenerator.h (right): https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/public/p... third_party/WebKit/public/platform/WebImageGenerator.h:44: static std::unique_ptr<SkImageGenerator> create(sk_sp<SkData>); On 2017/02/17 17:14:51, chrishtr wrote: > It's not clear to me immediately from digging into the Skia code what happens > with the pointer that > is returned from this. Does it cleanly obey unique_ptr semantics? Yes, that is why we changed the factory's signature. create() returns a ptr that is uniquely owned by the caller.
The CQ bit was checked by reed@google.com 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...
https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp (right): https://codereview.chromium.org/2706453003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp:331: std::unique_ptr<DecodingImageGenerator> generator(new DecodingImageGenerator( On 2017/02/17 16:41:08, f(malita) wrote: > nit: WTF::makeUnique(...); Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) 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 reed@google.com 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
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by reed@google.com 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: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
owners ptal
lgtm
lgtm
please reference a tracking bug from the CL description
Description was changed from ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG=skia:5077 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
On 2017/02/20 09:30:09, jochen wrote: > please reference a tracking bug from the CL description done
The CQ bit was checked by reed@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org Link to the patchset: https://codereview.chromium.org/2706453003/#ps40001 (title: "fix typo")
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
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
reed@google.com changed reviewers: + danakj@chromium.org, enne@chromium.org, vmpstr@chromium.org
need OWNER for cc/test
The CQ bit was checked by reed@google.com 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.
lgtm
The CQ bit was checked by reed@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org, jochen@chromium.org, fmalita@chromium.org Link to the patchset: https://codereview.chromium.org/2706453003/#ps50001 (title: "fix new site in tile_manager_unittest.cc")
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
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG=skia:5077 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG=skia:5077 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel TBR=hubbe ==========
The CQ bit was checked by reed@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 50001, "attempt_start_ts": 1487709832130670, "parent_rev": "dd47ba396451348cc15ecbc53255cb618e457414", "commit_rev": "08cd16372dba031d8adac493288fbe1a8c67a678"}
Message was sent while issue was closed.
Description was changed from ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG=skia:5077 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel TBR=hubbe ========== to ========== switch to new ImageGenerator API Since the image is the exclusive owner of the generator, change the parameter type to std::unique_ptr<SkImageGenerator> BUG=skia:5077 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel TBR=hubbe Review-Url: https://codereview.chromium.org/2706453003 Cr-Commit-Position: refs/heads/master@{#451848} Committed: https://chromium.googlesource.com/chromium/src/+/08cd16372dba031d8adac493288f... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:50001) as https://chromium.googlesource.com/chromium/src/+/08cd16372dba031d8adac493288f... |