tracing: fix edge case when dumping while trace being disabled

tracing: fix edge case when dumping while trace being disabled

A recent failure unveiled an edge case in memory-infra which causes
a crash if tracing is disabled between a RequestGlobalDump() and
a ContinueAsyncProcessDump().
This CL fixes the edge case and adds a test.

BUG=580295
Committed: https://crrev.com/5e13f78e61de954049966773407d2255087dc8a5
Cr-Commit-Position: refs/heads/master@{#371264}

[Primiano Tucci (use gerrit), 2016-01-22 13:18:41]

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

[ssid, 2016-01-22 15:31:14]

lgtm. Thanks

https://codereview.chromium.org/1620783002/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager.cc (left):

https://codereview.chromium.org/1620783002/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager.cc:483:
pmd_async_state->callback.Run(dump_guid, true /* success */);
This used to always return true and currently the only case where dump can fail
is when tracing is disabled.
Tracing could have been disabled just before calling or in between
FinalizeDumpAndAddToTrace and the trace events might not even get added in
trace. I think the logic here should be:
success = memory_tracing_enabled_ && pmd_async_state->dump_successful

I am fine with code as is.

[Primiano Tucci (use gerrit), 2016-01-25 16:29:39]

https://codereview.chromium.org/1620783002/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager.cc (left):

https://codereview.chromium.org/1620783002/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager.cc:483:
pmd_async_state->callback.Run(dump_guid, true /* success */);
On 2016/01/22 15:31:14, ssid wrote:
> This used to always return true and currently the only case where dump can
fail
> is when tracing is disabled.
> Tracing could have been disabled just before calling or in between
> FinalizeDumpAndAddToTrace and the trace events might not even get added in
> trace. I think the logic here should be:
> success = memory_tracing_enabled_ && pmd_async_state->dump_successful
> 
> I am fine with code as is.

You are right. Done.

[Primiano Tucci (use gerrit), 2016-01-25 16:29:42]

The CQ bit was checked by primiano@chromium.org

[Primiano Tucci (use gerrit), 2016-01-25 16:29:42]

The patchset sent to the CQ was uploaded after l-g-t-m from ssid@chromium.org
Link to the patchset: https://codereview.chromium.org/1620783002/#ps20001
(title: "final_check")

[commit-bot: I haz the power, 2016-01-25 16:30:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1620783002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1620783002/20001

[commit-bot: I haz the power, 2016-01-25 16:35:44]

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

[commit-bot: I haz the power, 2016-01-25 16:35:44]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Primiano Tucci (use gerrit), 2016-01-25 16:46:11]

The CQ bit was checked by primiano@chromium.org

[Primiano Tucci (use gerrit), 2016-01-25 16:46:11]

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

[commit-bot: I haz the power, 2016-01-25 16:46:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1620783002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1620783002/40001

[commit-bot: I haz the power, 2016-01-25 18:10:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-25 18:13:24]

Description was changed from

==========
tracing: fix edge case when dumping while trace being disabled

A recent failure unveiled an edge case in memory-infra which causes
a crash if tracing is disabled between a RequestGlobalDump() and
a ContinueAsyncProcessDump().
This CL fixes the edge case and adds a test.

BUG=580295
==========

to

==========
tracing: fix edge case when dumping while trace being disabled

A recent failure unveiled an edge case in memory-infra which causes
a crash if tracing is disabled between a RequestGlobalDump() and
a ContinueAsyncProcessDump().
This CL fixes the edge case and adds a test.

BUG=580295

Committed: https://crrev.com/5e13f78e61de954049966773407d2255087dc8a5
Cr-Commit-Position: refs/heads/master@{#371264}
==========

[commit-bot: I haz the power, 2016-01-25 18:13:25]

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