Umm, oysteine@ had already disagreed for this plan. The argument was that "the trace events should do anything only when tracing is enabled".
The CL you pointed out just does not add trace events to traces while recording should still be enabled (from startup) for heap dumps to show full information.

Once https://codereview.chromium.org/1923533004/ lands, could --enable-heap-profiling just enable tracing + filtering? Would that take care of the issue?

I have another (unsent) Cl on top of yours to make memory-infra work with filters. I think it adds more predicates for memory-infra filter.

I think all of primiano's comments on that CL got addressed; I'll ask simon to review it so we can land it, and then if primiano has more concerns when he gets back I'll fix in a followup.

It sounds like this concerns more than just DevTools.  Is another component more appropriate?

I am working on this, CL: https://codereview.chromium.org/2259493003/
I wonder how is devtools concerned about this? What are your plans?

If you have further plans after this cl, lets discuss about it.
If not please remove the Platform>DevTools>Memory label.

commit	f35fbb8a6b310e118dc2e04d29849f27613a4754	[log] [tgz]
author	ssid <ssid@chromium.org>	Thu Aug 25 00:04:31 2016
committer	Commit bot <commit-bot@chromium.org>	Thu Aug 25 00:06:37 2016
tree	0ef6b585f85102aa523401e4d51dcf0538b4a19f
parent	e85e6b6cf8d2a9bb95796cbe87efac18fc9689ff [diff]
[tracing] Add trace events filtering predicate for heap profiler

The heap profiler gets context for each allocation only from trace
events whose category is enabled for recording. This CL changes:
1. The AddTraceEvent was called only for trace events with categories
   that are enabled for recording modes, but now it gets called for all
   categories including ones specified in filters.
2. Trace events are not added to traces if the category is enabled for
   filtering but not enabled for recording, irrespective of the filter
   result.
3. Add new filter "heap_profiler_predicate" that tracks trace events and
   adds to heap profiler pseudo stack. This filter returns true only for
   the events enabled for recording mode (only the recording mode is
   affected by filters, not etw and event callback modes).
4. When "memory-infra" is enabled with heap profiler flag turned on,
   then the heap profiler filter is installed by default and filters all
   categories and adds to trace only if the category is enabled for
   recording.

This ensures that the heap profiler filter always gets all trace events
and they only get added to trace file if they are enabled for recording,
or they are enabled by other filters.

BUG= 598426 , 625170

Review-Url: https://codereview.chromium.org/2259493003
Cr-Commit-Position: refs/heads/master@{#414210}

bugdroid seems to have ignored this somehow.

There are 2 issues left:
1. For enabling memory infra using start-up tracing, the pseudo stack profiling does not work if filters are not added to trace config. This has to be worked around. TraceConfig should somehow add the filter if memory-infra category is enabled.

2. The events are recorded only when tracing is enabled. We need to find a way to always keep filtering on if heap profiling is turned on. It sounds like a simple change, but breaks the first rule of tracing: trace events should not do anything if tracing is not on. Can we just say tracing is on always if --enable-heap-profiling flag is passed?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e6ad8071badb6ce2bb83cc8278f9662f191c953

commit 7e6ad8071badb6ce2bb83cc8278f9662f191c953
Author: ssid <ssid@chromium.org>
Date: Wed Sep 21 00:27:50 2016

Fix races in trace events filtering.

The trace event filters for each category were stored as a list and the
objects were destroyed when tracing ends. But, when destroying these
filters the trace events on other thread which have checked the
ENABLED_FOR_FILTERING flag will try to access the filters list at the
same time. This CL:
1. Makes the filters enabled for categories a bitmap and restricts the
   number of filters that can be added in Chrome to 32 across all
   tracing sessions in a single browsing session. This removes the need
   for creation for filters for each category group.
2. Keeps all the filters alive till next tracing session starts since it
   is not possible to determine when the trace events have finished
   accessing the filter and it gives enough time for all the threads to
   complete acessing the filter.
3. Stores a global vector of all filters with max size 32 to access all
   the enabled filters for a trace category (corresponding to the bitmap
   flag) quickly.

This CL also fixes another issue where the trace events from V8 and Skia
were not recorded by EndEvent calls in filters. This is because the
EndFilter was called by trace_event.h. This call is now moved to
UpdateDuration, and end of all the events will now be recorded in
filters.

BUG=625170

Review-Url: https://codereview.chromium.org/2316403002
Cr-Commit-Position: refs/heads/master@{#419917}

[modify] https://crrev.com/7e6ad8071badb6ce2bb83cc8278f9662f191c953/base/trace_event/trace_event.h
[modify] https://crrev.com/7e6ad8071badb6ce2bb83cc8278f9662f191c953/base/trace_event/trace_log.cc
[modify] https://crrev.com/7e6ad8071badb6ce2bb83cc8278f9662f191c953/base/trace_event/trace_log.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20d2eeed2c735fbf018b9a9dffa810d51a25940d

commit 20d2eeed2c735fbf018b9a9dffa810d51a25940d
Author: ssid <ssid@chromium.org>
Date: Thu Oct 13 21:09:13 2016

[tracing] Add filtering mode in TraceLog

This CL does:
1. Add new FILTERING_MODE in TraceLog. TraceLog can be enabled and
   disabled with FILTERING_MODE independently from RECORDING MODE. The
   TraceLog::mode_ is a bitmap.
2. In FILTERING_MODE trace buffer is not created and trace events are
   passed to filters but not recorded.
3. The filters can be added only by FILTERING_MODE and trace config.
   Once enabled, the filters cannot be changed until the filters are
   disabled.
4. HeapProfilerFilter is enabled using FILTERING_MODE when
   "--enable-heap-profiling" flag is passed, so that pseudo stack is
   recorded for all allocations, irrespective of tracing.

BUG=625170

Review-Url: https://codereview.chromium.org/2323483005
Cr-Commit-Position: refs/heads/master@{#425156}

[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/heap_profiler_allocation_context_tracker_unittest.cc
[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/trace_config.cc
[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/trace_config.h
[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/trace_event_unittest.cc
[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/trace_log.cc
[modify] https://crrev.com/20d2eeed2c735fbf018b9a9dffa810d51a25940d/base/trace_event/trace_log.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ae272f6720806e97b1d3bfcb271e1c399dc15de

commit 6ae272f6720806e97b1d3bfcb271e1c399dc15de
Author: ssid <ssid@chromium.org>
Date: Thu Oct 13 22:28:07 2016

[tracing] Remove event callback mode in TraceLog

This is not used by any component, so cleaning up TraceLog.

BUG=625170

Review-Url: https://codereview.chromium.org/2354163002
Cr-Commit-Position: refs/heads/master@{#425192}

[modify] https://crrev.com/6ae272f6720806e97b1d3bfcb271e1c399dc15de/base/trace_event/trace_event.h
[modify] https://crrev.com/6ae272f6720806e97b1d3bfcb271e1c399dc15de/base/trace_event/trace_event_unittest.cc
[modify] https://crrev.com/6ae272f6720806e97b1d3bfcb271e1c399dc15de/base/trace_event/trace_log.cc
[modify] https://crrev.com/6ae272f6720806e97b1d3bfcb271e1c399dc15de/base/trace_event/trace_log.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77addc4fba2b9eb4354ba652b50e2c9cdb8475f3

commit 77addc4fba2b9eb4354ba652b50e2c9cdb8475f3
Author: ssid <ssid@chromium.org>
Date: Fri Oct 14 00:34:03 2016

Remove the NOTREACHED introduced by crrev.com/2323483005.

Causes webkit failures
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Precise__dbg_/603/layout-test-results/results.html
in
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Precise%20%28dbg%29/builds/603

BUG=625170
TBR=oysteine@chromium.org

Review-Url: https://codereview.chromium.org/2411333005
Cr-Commit-Position: refs/heads/master@{#425215}

[modify] https://crrev.com/77addc4fba2b9eb4354ba652b50e2c9cdb8475f3/base/trace_event/trace_log.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d0a5a15a80334107d48b415ce246bc1e6e4b147

commit 7d0a5a15a80334107d48b415ce246bc1e6e4b147
Author: ssid <ssid@chromium.org>
Date: Fri Oct 14 22:57:27 2016

Fix race in TraceLog::AddEventWithTimestamp

The enabled_modes_ cannot be accessed without lock. This CL changes
it to check if the current category is enabled to initialize thread
local.

Tsan build fail:
https://build.chromium.org/p/chromium.memory.full/builders/Linux%20TSan%20Tests/

BUG=625170
TBR=primiano@chromium.org

Review-Url: https://codereview.chromium.org/2421743003
Cr-Commit-Position: refs/heads/master@{#425500}

[modify] https://crrev.com/7d0a5a15a80334107d48b415ce246bc1e6e4b147/base/trace_event/trace_log.cc

This is mostly done, except for the NOTREACHED bug in #16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d2a3b2d3247797fd2d2c8f471b18f40b18cfeab

commit 2d2a3b2d3247797fd2d2c8f471b18f40b18cfeab
Author: ssid <ssid@chromium.org>
Date: Sat Mar 25 15:42:35 2017

Enable FILTERING_MODE for tracing if event filters were given in config

TracingControllerImpl, TraceMessageFilter and startup tracing code paths
enable FILTERING_MODE if event filters were given by trace_config. This
supports event filters in startup tracing and devtools.

BUG=625170

Review-Url: https://codereview.chromium.org/2466873002
Cr-Commit-Position: refs/heads/master@{#459653}

[modify] https://crrev.com/2d2a3b2d3247797fd2d2c8f471b18f40b18cfeab/components/tracing/child/child_trace_message_filter.cc
[modify] https://crrev.com/2d2a3b2d3247797fd2d2c8f471b18f40b18cfeab/components/tracing/child/child_trace_message_filter.h
[modify] https://crrev.com/2d2a3b2d3247797fd2d2c8f471b18f40b18cfeab/components/tracing/common/trace_startup.cc
[modify] https://crrev.com/2d2a3b2d3247797fd2d2c8f471b18f40b18cfeab/content/browser/tracing/tracing_controller_impl.cc
[modify] https://crrev.com/2d2a3b2d3247797fd2d2c8f471b18f40b18cfeab/content/browser/tracing/tracing_controller_impl.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e94989bc10b79261d67ccfeaf0e7e6308733747

commit 1e94989bc10b79261d67ccfeaf0e7e6308733747
Author: Siddhartha <ssid@chromium.org>
Date: Wed Sep 06 23:52:15 2017

Memory-infra: Heap profiler can be enabled when tracing

This is needed because new process starting when tracing and heap
profiler are enabled can get heap profiling notification later than
tracing enabled notification.
The heap profiler state should be setup when both tracing and heap
profiler are enabled. Do not call EnableHeapprofilingIfNeeded() from
constructor since the pseudo stack profiling needs to enable filtering
on TraceLog and MDM could be constructed by TraceLog in case of startup
tracing. This does not miss allocations since the heap profiling is
enabled by EnableStartupTracingIfNeeded very early (ContentMainRunner on
desktop and library loader on Android).

BUG= 757747 , 700245, 625170

Change-Id: I6fc593838dc1fa1c8450addefec782dd37ae9e5f
Reviewed-on: https://chromium-review.googlesource.com/651908
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500138}
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/heap_profiler_serialization_state.h
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/memory_dump_manager.h
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/memory_dump_manager_unittest.cc
[modify] https://crrev.com/1e94989bc10b79261d67ccfeaf0e7e6308733747/base/trace_event/process_memory_dump.cc