[tracing] Don't pop context from an empty stack.

[tracing] Don't pop context from an empty stack.

Small fix to avoid popping an empty stack. Tracing on Android crashes
on that almost every time.

Committed: https://crrev.com/9ea9470a13ec20a9480ae268af609599235ef4a2
Cr-Commit-Position: refs/heads/master@{#390744}

[Dmitry Skiba, 2016-04-28 16:02:26]

dskiba@google.com changed reviewers:
+ primiano@chromium.org, ssid@chromium.org

[ssid, 2016-04-29 08:19:58]

lgtm
why is the native stack tracing bug linked?
It'll be nice to have why it crashes and fix in the description.

[Primiano Tucci (use gerrit), 2016-04-29 12:02:36]

LGTM 

> why is the native stack tracing bug linked?
+1 this is about the pseudo stack

https://codereview.chromium.org/1927963003/diff/20001/base/trace_event/heap_p...
File base/trace_event/heap_profiler_allocation_context_tracker.cc (right):

https://codereview.chromium.org/1927963003/diff/20001/base/trace_event/heap_p...
base/trace_event/heap_profiler_allocation_context_tracker.cc:122:
DCHECK_EQ(context, task_contexts_.back())
ok following this logic (that is correcT) we should also remove this DCHECK then
(and the |context| argument alltogether)

[ssid, 2016-04-29 12:10:44]

https://codereview.chromium.org/1927963003/diff/20001/base/trace_event/heap_p...
File base/trace_event/heap_profiler_allocation_context_tracker.cc (right):

https://codereview.chromium.org/1927963003/diff/20001/base/trace_event/heap_p...
base/trace_event/heap_profiler_allocation_context_tracker.cc:122:
DCHECK_EQ(context, task_contexts_.back())
On 2016/04/29 12:02:36, Primiano Tucci (traveling) wrote:
> ok following this logic (that is correcT) we should also remove this DCHECK
then
> (and the |context| argument alltogether)

This check is still valid because the contexts cannot be misordered. Either the
context can go missing which means the stack will be empty or the context should
match the top.

This is unlike trace event because once the enabled flag is set it's never
unset. while tracing can be disabled and events can miss ordering in pseudo
stack.

[Primiano Tucci (use gerrit), 2016-04-29 12:20:54]

On 2016/04/29 12:10:44, ssid wrote:
>
https://codereview.chromium.org/1927963003/diff/20001/base/trace_event/heap_p...
> File base/trace_event/heap_profiler_allocation_context_tracker.cc (right):
> 
>
https://codereview.chromium.org/1927963003/diff/20001/base/trace_event/heap_p...
> base/trace_event/heap_profiler_allocation_context_tracker.cc:122:
> DCHECK_EQ(context, task_contexts_.back())
> On 2016/04/29 12:02:36, Primiano Tucci (traveling) wrote:
> > ok following this logic (that is correcT) we should also remove this DCHECK
> then
> > (and the |context| argument alltogether)
> 
> This check is still valid because the contexts cannot be misordered. Either
the
> context can go missing which means the stack will be empty or the context
should
> match the top.

Oh you make a good point. I was thinking to the case of [BEGIN, enable
profiling, END], but that is the case of task_contexts_.empty() is going to
catch. 
Nevermind then, just LGTM without comments :)

[Dmitry Skiba, 2016-04-29 19:35:02]

Description was changed from

==========
[tracing] Don't pop context from an empty stack.

Small fix to avoid popping an empty stack. Tracing on Android crashes
on that almost every time.

BUG=602701
==========

to

==========
[tracing] Don't pop context from an empty stack.

Small fix to avoid popping an empty stack. Tracing on Android crashes
on that almost every time.
==========

[Dmitry Skiba, 2016-04-29 19:35:59]

The CQ bit was checked by dskiba@google.com

[commit-bot: I haz the power, 2016-04-29 19:36:14]

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

[commit-bot: I haz the power, 2016-04-29 20:35:44]

Description was changed from

==========
[tracing] Don't pop context from an empty stack.

Small fix to avoid popping an empty stack. Tracing on Android crashes
on that almost every time.
==========

to

==========
[tracing] Don't pop context from an empty stack.

Small fix to avoid popping an empty stack. Tracing on Android crashes
on that almost every time.
==========

[commit-bot: I haz the power, 2016-04-29 20:35:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-30 17:28:32]

Patchset 2 (id:??) landed as
https://crrev.com/9ea9470a13ec20a9480ae268af609599235ef4a2
Cr-Commit-Position: refs/heads/master@{#390744}

[commit-bot: I haz the power, 2016-04-30 18:02:20]

Description was changed from

==========
[tracing] Don't pop context from an empty stack.

Small fix to avoid popping an empty stack. Tracing on Android crashes
on that almost every time.
==========

to

==========
[tracing] Don't pop context from an empty stack.

Small fix to avoid popping an empty stack. Tracing on Android crashes
on that almost every time.

Committed: https://crrev.com/9ea9470a13ec20a9480ae268af609599235ef4a2
Cr-Commit-Position: refs/heads/master@{#390744}
==========