SkBitmap and SkPixelRef no longer need lock/unlock

SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

refactoring CL
TBR=jochen, hubbe, finnur, thestig, wez

Review-Url: https://codereview.chromium.org/2823003002
Cr-Commit-Position: refs/heads/master@{#465770}
Committed: https://chromium.googlesource.com/chromium/src/+/11395368450c1bff110eba0cafef112c2f0e8334

[reed1, 2017-04-17 18:33:22]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

BUG=skia:6481
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[reed1, 2017-04-17 18:33:28]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

[reed1, 2017-04-17 18:33:32]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

[reed1, 2017-04-17 18:36:24]

The CQ bit was checked by reed@google.com to run a CQ dry run

[reed1, 2017-04-17 18:36:41]

reed@google.com changed reviewers:
+ fmalita@chromium.org

[commit-bot: I haz the power, 2017-04-17 18:36:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 18:46:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-17 18:46:51]

Dry run: 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...)

[reed1, 2017-04-17 18:58:36]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 18:58:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 19:09:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-17 19:09:19]

Dry run: 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...)

[f(malita), 2017-04-17 19:31:36]

non-owner lgtm

https://codereview.chromium.org/2823003002/diff/40001/cc/resources/ui_resourc...
File cc/resources/ui_resource_bitmap.h (right):

https://codereview.chromium.org/2823003002/diff/40001/cc/resources/ui_resourc...
cc/resources/ui_resource_bitmap.h:58: return pixel_ref_ ? static_cast<const
uint8_t*>(pixel_ref_->pixels())
nit: prev version assumes pixel_ref_ is non-null; maybe DCHECK_NE to document
instead of explicit check?

[reed1, 2017-04-17 20:18:40]

https://codereview.chromium.org/2823003002/diff/40001/cc/resources/ui_resourc...
File cc/resources/ui_resource_bitmap.h (right):

https://codereview.chromium.org/2823003002/diff/40001/cc/resources/ui_resourc...
cc/resources/ui_resource_bitmap.h:58: return pixel_ref_ ? static_cast<const
uint8_t*>(pixel_ref_->pixels())
On 2017/04/17 19:31:36, f(malita) wrote:
> nit: prev version assumes pixel_ref_ is non-null; maybe DCHECK_NE to document
> instead of explicit check?

removed check.

[reed1, 2017-04-17 20:18:48]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 20:19:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 20:23:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-17 20:23:49]

Dry run: Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[reed1, 2017-04-17 20:56:49]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 20:57:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 21:07:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-17 21:07:22]

Dry run: 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...)

[reed1, 2017-04-17 21:54:50]

reed@google.com changed reviewers:
+ dcheng@chromium.org, rsesek@chromium.org

[reed1, 2017-04-17 21:54:51]

adding security OWNERS

[reed1, 2017-04-17 21:55:22]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

[dcheng, 2017-04-17 22:03:10]

ipc lgtm with nits addressed

(in the future though, please just pick one IPC reviewer)

https://codereview.chromium.org/2823003002/diff/80001/skia/public/interfaces/...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):

https://codereview.chromium.org/2823003002/diff/80001/skia/public/interfaces/...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:202: void*
StructTraits<skia::mojom::BitmapDataView, SkBitmap>::SetUpContext(
Nit: please remove these members, since they're no longer needed.

[reed1, 2017-04-18 12:53:02]

https://codereview.chromium.org/2823003002/diff/80001/skia/public/interfaces/...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):

https://codereview.chromium.org/2823003002/diff/80001/skia/public/interfaces/...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:202: void*
StructTraits<skia::mojom::BitmapDataView, SkBitmap>::SetUpContext(
On 2017/04/17 22:03:10, dcheng (OOO through May 2) wrote:
> Nit: please remove these members, since they're no longer needed.

Done.

[reed1, 2017-04-18 12:53:12]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 12:53:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 13:03:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 13:03:46]

Dry run: 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...)

[reed1, 2017-04-18 14:18:08]

reed@google.com changed reviewers:
+ danakj@chromium.org

[reed1, 2017-04-18 14:18:09]

owner for ui/gfx cc/

[danakj, 2017-04-18 14:29:28]

LGTM

[reed1, 2017-04-18 15:14:24]

The CQ bit was checked by reed@google.com

[reed1, 2017-04-18 15:14:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
fmalita@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2823003002/#ps30106
(title: "remove empty and unused methods")

[commit-bot: I haz the power, 2017-04-18 15:14:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 15:24:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 15:24:14]

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...)

[reed1, 2017-04-18 15:52:37]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

TBR=sky
pure refactoring/deleting-of-empty-functions
==========

[reed1, 2017-04-18 15:52:49]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2017-04-18 15:53:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 16:02:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 16:02:52]

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...)

[reed1, 2017-04-18 16:05:44]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

TBR=sky
pure refactoring/deleting-of-empty-functions
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

TBR=sky
==========

[reed1, 2017-04-18 16:05:44]

reed@google.com changed reviewers:
+ rjkroege@chromium.org, sky@chromium.org

[reed1, 2017-04-18 16:06:02]

security OWNER

[sky, 2017-04-18 16:25:24]

LGTM

[reed1, 2017-04-18 17:24:24]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

TBR=sky
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

[reed1, 2017-04-18 17:24:36]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2017-04-18 17:25:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 17:36:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 17:36:09]

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...)

[reed1, 2017-04-19 01:28:21]

need SECURITY_OWNER

[Robert Sesek, 2017-04-19 15:00:39]

On 2017/04/19 01:28:21, reed1 wrote:
> need SECURITY_OWNER

dcheng@ qualifies. The presubmit is telling you to add an OWNERS file:


Found changes to IPC files without a security OWNER!

***************
ui/gfx/ipc/skia/OWNERS is missing the following lines:

per-file *_param_traits*.*=set noparent
per-file *_param_traits*.*=file://ipc/SECURITY_OWNERS

for changed files:
  ui/gfx/ipc/skia/gfx_skia_param_traits.cc
***************

[reed1, 2017-04-19 17:57:23]

The CQ bit was checked by reed@google.com

[reed1, 2017-04-19 17:57:23]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
dcheng@chromium.org, danakj@chromium.org, fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/2823003002/#ps110001
(title: "add new OWNERS file")

[commit-bot: I haz the power, 2017-04-19 17:58:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reed1, 2017-04-19 17:59:12]

On 2017/04/19 15:00:39, Robert Sesek wrote:
> On 2017/04/19 01:28:21, reed1 wrote:
> > need SECURITY_OWNER
> 
> dcheng@ qualifies. The presubmit is telling you to add an OWNERS file:
> 
> 
> Found changes to IPC files without a security OWNER!
> 
> ***************
> ui/gfx/ipc/skia/OWNERS is missing the following lines:
> 
> per-file *_param_traits*.*=set noparent
> per-file *_param_traits*.*=file://ipc/SECURITY_OWNERS
> 
> for changed files:
>   ui/gfx/ipc/skia/gfx_skia_param_traits.cc
> ***************

Thanks for clarifying.

[commit-bot: I haz the power, 2017-04-19 18:03:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 18:03:23]

Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[reed1, 2017-04-19 18:17:34]

The CQ bit was checked by reed@google.com

[reed1, 2017-04-19 18:17:35]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
dcheng@chromium.org, danakj@chromium.org, fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/2823003002/#ps130001
(title: "manual rebase")

[commit-bot: I haz the power, 2017-04-19 18:18:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 18:35:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 18:35:37]

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...)

[reed1, 2017-04-19 18:44:13]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

refactoring CL
TBR=jochen, hubbe, finnur, thestig, wez
==========

[reed1, 2017-04-19 18:44:27]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2017-04-19 18:44:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 19:44:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 19:44:31]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[reed1, 2017-04-19 20:08:08]

The CQ bit was checked by reed@google.com

[reed1, 2017-04-19 20:08:09]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
dcheng@chromium.org, danakj@chromium.org, fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/2823003002/#ps150001
(title: "win fix")

[commit-bot: I haz the power, 2017-04-19 20:08:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 22:06:00]

CQ is committing da patch.

Bot data: {"patchset_id": 150001, "attempt_start_ts": 1492632488353180,
"parent_rev": "688b9d10d09318fe00907e50d08c60a517f9ddc0", "commit_rev":
"a54013c3e93debe124fcfce0682e185b0ba55e86"}

[commit-bot: I haz the power, 2017-04-19 22:06:17]

CQ is committing da patch.

Bot data: {"patchset_id": 150001, "attempt_start_ts": 1492632488353180,
"parent_rev": "d920c93c9c963460b835d19ca4540fe86640b19f", "commit_rev":
"11395368450c1bff110eba0cafef112c2f0e8334"}

[commit-bot: I haz the power, 2017-04-19 22:06:52]

Description was changed from

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

refactoring CL
TBR=jochen, hubbe, finnur, thestig, wez
==========

to

==========
SkBitmap and SkPixelRef no longer need lock/unlock

Refactoring CL -- no intended behavior change. Just removing calls to empty
apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

refactoring CL
TBR=jochen, hubbe, finnur, thestig, wez

Review-Url: https://codereview.chromium.org/2823003002
Cr-Commit-Position: refs/heads/master@{#465770}
Committed:
https://chromium.googlesource.com/chromium/src/+/11395368450c1bff110eba0cafef...
==========

[commit-bot: I haz the power, 2017-04-19 22:06:55]

Committed patchset #9 (id:150001) as
https://chromium.googlesource.com/chromium/src/+/11395368450c1bff110eba0cafef...