[tracing] Add memory usage of Android UI resources to memory-infra

[tracing] Add memory usage of Android UI resources to memory-infra

Add provider to track memory usage of bitmap resources in browser.

BUG=654640
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/49a669c8ddfd8a45ab0318798359b9b08d2d9fa8
Cr-Commit-Position: refs/heads/master@{#425232}

[ssid, 2016-10-11 01:50:15]

Description was changed from

==========
[tracing] Add memory usage of Android UI resources to memory-infra

Add provider to track memory usage of bitmap resources in browser.

BUG=654640
==========

to

==========
[tracing] Add memory usage of Android UI resources to memory-infra

Add provider to track memory usage of bitmap resources in browser.

BUG=654640
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[ssid, 2016-10-11 01:58:26]

Patchset #1 (id:1) has been deleted

[ssid, 2016-10-11 20:20:40]

Patchset #1 (id:20001) has been deleted

[ssid, 2016-10-11 20:21:02]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 20:21:42]

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

[ssid, 2016-10-11 20:25:03]

Patchset #1 (id:40001) has been deleted

[ssid, 2016-10-11 20:25:42]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 20:26:13]

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

[ssid, 2016-10-11 20:30:23]

ssid@chromium.org changed reviewers:
+ tedchoc@chromium.org

[ssid, 2016-10-11 20:30:23]

ptal, thanks!

[Ted C, 2016-10-11 20:48:43]

tedchoc@chromium.org changed reviewers:
+ dtrainor@chromium.org

[Ted C, 2016-10-11 20:48:44]

+dtrainor as he's a better owner for ui/android/resources

[commit-bot: I haz the power, 2016-10-11 21:19:57]

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

[commit-bot: I haz the power, 2016-10-11 21:19:58]

Dry run: This issue passed the CQ dry run.

[Dmitry Skiba, 2016-10-13 03:47:02]

dskiba@google.com changed reviewers:
+ dskiba@google.com

[Dmitry Skiba, 2016-10-13 03:47:03]

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/cr...
File ui/android/resources/crushed_sprite_resource.cc (right):

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/cr...
ui/android/resources/crushed_sprite_resource.cc:64: return bitmap_.getSize() +
last_frame_resource_
I would just remove last_frame_resource_, it's not set anywhere.
Also, this formatting is misleading - it reads like (bitmap.getSize() +
last_frame_resource_) ? ... : 0;

[David Trainor- moved to gerrit, 2016-10-13 04:07:21]

lgtm % nits!  Thanks :).

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

https://codereview.chromium.org/2406103002/diff/80001/cc/resources/ui_resourc...
cc/resources/ui_resource_bitmap.h:56: return pixel_ref_ ? pixel_ref_->rowBytes()
* size_.height() : 0;
Would pixel_ref_->getAllocatedSizeInBytes() be more correct?  Not sure, just
something to look into.

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/re...
File ui/android/resources/resource_manager_impl_unittest.cc (right):

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/re...
ui/android/resources/resource_manager_impl_unittest.cc:189: new
base::trace_event::ProcessMemoryDump(nullptr, dump_args));
MakeUnique

[ssid, 2016-10-13 17:45:16]

Thanks made changes.

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

https://codereview.chromium.org/2406103002/diff/80001/cc/resources/ui_resourc...
cc/resources/ui_resource_bitmap.h:56: return pixel_ref_ ? pixel_ref_->rowBytes()
* size_.height() : 0;
On 2016/10/13 04:07:21, David Trainor wrote:
> Would pixel_ref_->getAllocatedSizeInBytes() be more correct?  Not sure, just
> something to look into.

Done.

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/cr...
File ui/android/resources/crushed_sprite_resource.cc (right):

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/cr...
ui/android/resources/crushed_sprite_resource.cc:64: return bitmap_.getSize() +
last_frame_resource_
On 2016/10/13 03:47:03, Dmitry Skiba wrote:
> I would just remove last_frame_resource_, it's not set anywhere.
> Also, this formatting is misleading - it reads like (bitmap.getSize() +
> last_frame_resource_) ? ... : 0;

Done.

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/re...
File ui/android/resources/resource_manager_impl_unittest.cc (right):

https://codereview.chromium.org/2406103002/diff/80001/ui/android/resources/re...
ui/android/resources/resource_manager_impl_unittest.cc:189: new
base::trace_event::ProcessMemoryDump(nullptr, dump_args));
On 2016/10/13 04:07:21, David Trainor wrote:
> MakeUnique

Done.

[ssid, 2016-10-13 17:46:06]

+Eric ptal, thanks.

[ssid, 2016-10-13 17:46:52]

ssid@chromium.org changed reviewers:
+ ericrk@chromium.org

[ssid, 2016-10-13 17:46:53]

+Eric ptal, thanks.

[ssid, 2016-10-13 18:54:48]

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

https://codereview.chromium.org/2406103002/diff/80001/cc/resources/ui_resourc...
cc/resources/ui_resource_bitmap.h:56: return pixel_ref_ ? pixel_ref_->rowBytes()
* size_.height() : 0;
On 2016/10/13 04:07:21, David Trainor wrote:
> Would pixel_ref_->getAllocatedSizeInBytes() be more correct?  Not sure, just
> something to look into.

Sorry I misunderstood. Yes, I am planning to make this method public from Skia
side, I will clean up this part when that rolls.

[ericrk, 2016-10-14 00:07:38]

lgtm

https://codereview.chromium.org/2406103002/diff/120001/cc/resources/ui_resour...
File cc/resources/ui_resource_bitmap.h (right):

https://codereview.chromium.org/2406103002/diff/120001/cc/resources/ui_resour...
cc/resources/ui_resource_bitmap.h:56: return pixel_ref_ ? pixel_ref_->rowBytes()
* size_.height() : 0;
I think this is fine - in some other places we use something like:

pixel_ref_->info().getSafeSize(pixel_ref_->rowBytes());

But I don't think it matters here.

[ssid, 2016-10-14 00:57:24]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 00:57:37]

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

[David Trainor- moved to gerrit, 2016-10-14 01:35:32]

https://codereview.chromium.org/2406103002/diff/120001/cc/resources/ui_resour...
File cc/resources/ui_resource_bitmap.h (right):

https://codereview.chromium.org/2406103002/diff/120001/cc/resources/ui_resour...
cc/resources/ui_resource_bitmap.h:56: return pixel_ref_ ? pixel_ref_->rowBytes()
* size_.height() : 0;
On 2016/10/14 00:07:38, ericrk wrote:
> I think this is fine - in some other places we use something like:
> 
> pixel_ref_->info().getSafeSize(pixel_ref_->rowBytes());
> 
> But I don't think it matters here.

Ah yeah sorry I didn't realize the other method wasn't exposed.  Either of these
are probably good enough :).

[ssid, 2016-10-14 01:38:23]

https://codereview.chromium.org/2406103002/diff/120001/cc/resources/ui_resour...
File cc/resources/ui_resource_bitmap.h (right):

https://codereview.chromium.org/2406103002/diff/120001/cc/resources/ui_resour...
cc/resources/ui_resource_bitmap.h:56: return pixel_ref_ ? pixel_ref_->rowBytes()
* size_.height() : 0;
On 2016/10/14 01:35:31, David Trainor wrote:
> On 2016/10/14 00:07:38, ericrk wrote:
> > I think this is fine - in some other places we use something like:
> > 
> > pixel_ref_->info().getSafeSize(pixel_ref_->rowBytes());
> > 
> > But I don't think it matters here.
> 
> Ah yeah sorry I didn't realize the other method wasn't exposed.  Either of
these
> are probably good enough :).

Okay keeping it as is. I will ping Skia owners to expose a method. If it's done
I will fix this.
Thanks.

[commit-bot: I haz the power, 2016-10-14 01:42:41]

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

[commit-bot: I haz the power, 2016-10-14 01:42:42]

Dry run: This issue passed the CQ dry run.

[ssid, 2016-10-14 01:43:16]

The CQ bit was checked by ssid@chromium.org

[ssid, 2016-10-14 01:43:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2406103002/#ps120001
(title: "Fixes.")

[commit-bot: I haz the power, 2016-10-14 01:43:45]

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, 2016-10-14 01:50:07]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2016-10-14 01:52:37]

Description was changed from

==========
[tracing] Add memory usage of Android UI resources to memory-infra

Add provider to track memory usage of bitmap resources in browser.

BUG=654640
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
[tracing] Add memory usage of Android UI resources to memory-infra

Add provider to track memory usage of bitmap resources in browser.

BUG=654640
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/49a669c8ddfd8a45ab0318798359b9b08d2d9fa8
Cr-Commit-Position: refs/heads/master@{#425232}
==========

[commit-bot: I haz the power, 2016-10-14 01:52:38]

Patchset 4 (id:??) landed as
https://crrev.com/49a669c8ddfd8a45ab0318798359b9b08d2d9fa8
Cr-Commit-Position: refs/heads/master@{#425232}