memory coordinator: Add trace macros to track state changes

memory coordinator: Add trace macros to track state changes

BUG=617492
Committed: https://crrev.com/dd32a4ec69b482248bc23df10cc0fd6efc56ddec
Cr-Commit-Position: refs/heads/master@{#428949}

[bashi, 2016-11-01 03:23:55]

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

[commit-bot: I haz the power, 2016-11-01 03:24:14]

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

[bashi, 2016-11-01 03:25:07]

bashi@chromium.org changed reviewers:
+ dcheng@chromium.org, haraken@chromium.org

[bashi, 2016-11-01 03:25:08]

PTAL?

haraken@: content/{child,browser}/memory
dcheng@: base/

[haraken, 2016-11-01 03:58:06]

https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
content/browser/memory/memory_coordinator_impl.cc:202: "next",
static_cast<int32_t>(next_state));

Is there any way to pass in human-readable strings to trace events?

[bashi, 2016-11-01 04:04:40]

https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
content/browser/memory/memory_coordinator_impl.cc:202: "next",
static_cast<int32_t>(next_state));
On 2016/11/01 03:58:06, haraken wrote:
> 
> Is there any way to pass in human-readable strings to trace events?

I'm happy to add a helper function for MemoryState -> string conversion. Do we
want it?

[haraken, 2016-11-01 04:05:16]

On 2016/11/01 04:04:40, bashi1 (slow) wrote:
>
https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
> File content/browser/memory/memory_coordinator_impl.cc (right):
> 
>
https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
> content/browser/memory/memory_coordinator_impl.cc:202: "next",
> static_cast<int32_t>(next_state));
> On 2016/11/01 03:58:06, haraken wrote:
> > 
> > Is there any way to pass in human-readable strings to trace events?
> 
> I'm happy to add a helper function for MemoryState -> string conversion. Do we
> want it?

Yeah, that sounds better to me.

[bashi, 2016-11-01 04:17:21]

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

[bashi, 2016-11-01 04:17:27]

https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
File content/browser/memory/memory_coordinator_impl.cc (right):

https://codereview.chromium.org/2466773002/diff/1/content/browser/memory/memo...
content/browser/memory/memory_coordinator_impl.cc:202: "next",
static_cast<int32_t>(next_state));
On 2016/11/01 04:04:40, bashi1 (slow) wrote:
> On 2016/11/01 03:58:06, haraken wrote:
> > 
> > Is there any way to pass in human-readable strings to trace events?
> 
> I'm happy to add a helper function for MemoryState -> string conversion. Do we
> want it?

Done.

[commit-bot: I haz the power, 2016-11-01 04:17:35]

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

[haraken, 2016-11-01 04:20:29]

LGTM

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_client.cc (right):

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.cc:7: #include "base/logging.h"

Do we need this?

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.cc:27: 

Unnecessary empty line.

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_client.h (right):

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.h:28: enum class MemoryState : int32_t {

This change wouldn't be needed.

[commit-bot: I haz the power, 2016-11-01 04:28:32]

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

[commit-bot: I haz the power, 2016-11-01 04:28:33]

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

[dcheng, 2016-11-01 04:33:37]

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_client.h (right):

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.h:45: BASE_EXPORT std::string
MemoryStateToString(MemoryState state);
If possible, let's return const char* to make this a bit more efficient.

[bashi, 2016-11-01 05:02:05]

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

[commit-bot: I haz the power, 2016-11-01 05:02:14]

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

[bashi, 2016-11-01 05:02:16]

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_client.cc (right):

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.cc:7: #include "base/logging.h"
On 2016/11/01 04:20:29, haraken wrote:
> 
> Do we need this?

NOTREACHED()

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.cc:27: 
On 2016/11/01 04:20:29, haraken wrote:
> 
> Unnecessary empty line.

Done.

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
File base/memory/memory_coordinator_client.h (right):

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.h:28: enum class MemoryState : int32_t {
On 2016/11/01 04:20:29, haraken wrote:
> 
> This change wouldn't be needed.

Done.

https://codereview.chromium.org/2466773002/diff/20001/base/memory/memory_coor...
base/memory/memory_coordinator_client.h:45: BASE_EXPORT std::string
MemoryStateToString(MemoryState state);
On 2016/11/01 04:33:37, dcheng wrote:
> If possible, let's return const char* to make this a bit more efficient.

Done.

[dcheng, 2016-11-01 05:58:21]

base LGTM

[commit-bot: I haz the power, 2016-11-01 06:04:50]

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

[commit-bot: I haz the power, 2016-11-01 06:04:50]

Dry run: This issue passed the CQ dry run.

[bashi, 2016-11-01 06:32:41]

The CQ bit was checked by bashi@chromium.org

[bashi, 2016-11-01 06:32:42]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2466773002/#ps40001
(title: "comment")

[commit-bot: I haz the power, 2016-11-01 06:32:57]

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-11-01 06:36:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-01 06:38:31]

Description was changed from

==========
memory coordinator: Add trace macros to track state changes

BUG=617492
==========

to

==========
memory coordinator: Add trace macros to track state changes

BUG=617492

Committed: https://crrev.com/dd32a4ec69b482248bc23df10cc0fd6efc56ddec
Cr-Commit-Position: refs/heads/master@{#428949}
==========

[commit-bot: I haz the power, 2016-11-01 06:38:32]

Patchset 3 (id:??) landed as
https://crrev.com/dd32a4ec69b482248bc23df10cc0fd6efc56ddec
Cr-Commit-Position: refs/heads/master@{#428949}