Reland of [tracing] Simplify logic of MemoryDumpManager
Reason for revert:
Original CL: crrev.com/1536533004
Revert: crrev.com/1540283003
Reason for reland:
The CL was reverted because caused a LayoutTest failure
(pending-version-change-stuck-works-with-terminate.html).
The failure was due to a race condition in an unrelated part of the
codebase (see crbug.com/571432). The original CL managed to just
unveil reliably the race. The race condition has been fixed in
crrev.com/1544663002. I verified that the LayoutTest doesn't crash
locally anymore.
Original issue's description:
> Revert of [tracing] Simplify logic of MemoryDumpManager (patchset #9 id:160001 of https://codereview.chromium.org/1536533004/ )
>
> Reason for revert:
> Caused crash in pending-version-change-stuck-works-with-terminate.html layout test.
>
> See:
> https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.10/builds/9506/steps/webkit_tests/logs/stdio
>
> Original issue's description:
> > [tracing] Simplify logic of MemoryDumpManager
> >
> > This CL simplifies the unregistration logic of MemoryDumpManager.
> > This is to make the overall code more readable and simplify upcoming
> > changes.
> > Prior to this CL, the MemoryDumpManager was keeping only one list to
> > keep track of the registered dump providers. This caused the
> > unregistration logic to be tricky because:
> > - We couldn't remove the MDPInfo straight away, as that might
> > cause invalidation of the |next_dump_provider| iterator.
> > - We had to flag the MDPInfo as unregistered and postpone the deletion
> > on the next dump.
> > This has a major drawback: if we keep registering and unregistering
> > dump providers when no tracing is happening at all, the dump_providers_
> > list grows without bounds. This is bad.
> >
> > This CL changes the logic as follows:
> > - MDPInfo becomes refcounted. At any time it can be referenced by:
> > - The MDM's |dump_providers_| list.
> > - The |ProcessMemoryDumpAsyncState.pending_dump_providers| list.
> > - Upon each dump, the dump provider list is snapshotted in the
> > |ProcessMemoryDumpAsyncState.pending_dump_providers|
> > - Upon unregistration of a dump provider we just remove the MDPInfo
> > from the MDM's |dump_providers_|. If a dump is ongoing, the
> > ProcessMemoryDumpAsyncState will keep it refcounted and delete it
> > during the dump.
> >
> > This CL does not add or change any behavior of the MemoryDumpManager,
> > with the exception of:
> > - Fixing a corner case when dumping with no dump providers registered
> > (See changes in the unittest).
> > - Making the fail-safe logic more strict: if a dumper fails once, it
> > will stay disabled forever.
> >
> > BUG=461788
> >
> > Committed: https://crrev.com/9734733909e7cb41ef5c153f3c2d0927e8209133
> > Cr-Commit-Position: refs/heads/master@{#366374}
>
> TBR=ruuda@google.com,ssid@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=461788
>
> Committed: https://crrev.com/6a7365c4c5ad3e3083ce3d9a269c3e1d8fdb35bc
> Cr-Commit-Position: refs/heads/master@{#366386}
TBR=ruuda@google.com,ssid@chromium.org
BUG=461788
Review URL: https://codereview.chromium.org/1546583002
Cr-Commit-Position: refs/heads/master@{#366588}
CrOS-Libchrome-Original-Commit: 6dfc15bc20d12fdcd401011b00a521c4e557386e
3 files changed