Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(657)

Issue 1042723002: [tracing] IPC messages and stubs for inter-process memory dumps (Closed)

Created:
5 years, 8 months ago by Primiano Tucci (use gerrit)
Modified:
5 years, 8 months ago
Reviewers:
kenrb, nduca, Sami
CC:
chromium-reviews, darin-cc_chromium.org, jam, tracing+reviews_chromium.org, wfh+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@ipc_2_delegate
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[tracing] IPC messages and stubs for inter-process memory dumps This CL introduces the four IPC messages and the message filter stubs that will be used by the upcoming CLs to coordinate memory dumps across processes. The global coordination policy is simple: all the global dumps must be orchestrated and handled by the browser process' MemoryDumpManager. Memory dumps initiated by a child process must be routed through the browser's MDM. In other words this reads as: all the Chrome processes have a MDM, but the browser's MDM is a more senior and responsible manager. This CL introduces a total of four IPC messages: (request, response) x (process, global), as follows: Request local (i.e. current process) dumps to children: (browser -> child) TracingMsg_ProcessMemoryDumpRequest (child -> browser) TracingHostMsg_ProcessMemoryDumpResponse Initiate global dumps from a child process: (child -> browser) TracingHostMsg_GlobalMemoryDumpRequest (browser -> child) TracingMsg_GlobalMemoryDumpResponse If the global dump is initiated by the browser process, a total of N_children x 2 (req/resp) IPC messages occur to perform the dump. If the global dump is initiated by a child process, a further couple of IPC messages (GlobalMemoryDump{Request,Response}) is required in order to, respectively, tell the browser's MDM to initiate the dump and get the final result back (all the child processes succeed / the global dump was aborted because another one was in progress). More context and design doc are available in the attached BUG. BUG=462930 Committed: https://crrev.com/04f9de655915bfaf9fa2c90ee4848452ca71d89f Cr-Commit-Position: refs/heads/master@{#323284}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : Rebase #

Total comments: 11

Patch Set 4 : Rebase + skyostil@ review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -2 lines) Patch
M components/tracing/child_trace_message_filter.h View 1 2 3 chunks +8 lines, -0 lines 0 comments Download
M components/tracing/child_trace_message_filter.cc View 1 2 3 2 chunks +33 lines, -0 lines 0 comments Download
M components/tracing/tracing_messages.h View 1 2 4 chunks +29 lines, -0 lines 0 comments Download
M content/browser/tracing/trace_message_filter.h View 1 2 3 chunks +8 lines, -0 lines 0 comments Download
M content/browser/tracing/trace_message_filter.cc View 1 2 3 3 chunks +29 lines, -0 lines 0 comments Download
M content/browser/tracing/tracing_controller_impl.h View 1 2 4 chunks +12 lines, -2 lines 0 comments Download
M content/browser/tracing/tracing_controller_impl.cc View 1 2 3 2 chunks +38 lines, -0 lines 0 comments Download

Messages

Total messages: 17 (6 generated)
Primiano Tucci (use gerrit)
This is a set of 5 CLs (+ another couple for tests) to implement IPC ...
5 years, 8 months ago (2015-03-30 19:54:18 UTC) #2
nduca
non-exhaustive owner lgtm. would be good to get someone to second-eye on the details.
5 years, 8 months ago (2015-03-30 20:01:49 UTC) #4
Primiano Tucci (use gerrit)
+kenrb for components/tracing/tracing_messages.h +skyostil for a general review
5 years, 8 months ago (2015-03-31 14:40:30 UTC) #5
Sami
lgtm with a couple of nits. https://codereview.chromium.org/1042723002/diff/40001/components/tracing/child_trace_message_filter.cc File components/tracing/child_trace_message_filter.cc (right): https://codereview.chromium.org/1042723002/diff/40001/components/tracing/child_trace_message_filter.cc#newcode200 components/tracing/child_trace_message_filter.cc:200: // the browser ...
5 years, 8 months ago (2015-03-31 14:54:36 UTC) #7
Primiano Tucci (use gerrit)
+kenrb for components/tracing/tracing_messages.h
5 years, 8 months ago (2015-03-31 23:24:58 UTC) #9
Primiano Tucci (use gerrit)
https://codereview.chromium.org/1042723002/diff/40001/components/tracing/child_trace_message_filter.cc File components/tracing/child_trace_message_filter.cc (right): https://codereview.chromium.org/1042723002/diff/40001/components/tracing/child_trace_message_filter.cc#newcode200 components/tracing/child_trace_message_filter.cc:200: // the browser and clear up the bookeeping. On ...
5 years, 8 months ago (2015-04-01 11:05:41 UTC) #10
kenrb
ipc lgtm
5 years, 8 months ago (2015-04-01 15:53:54 UTC) #11
Sami
Thanks, lgtm. https://codereview.chromium.org/1042723002/diff/40001/components/tracing/tracing_messages.h File components/tracing/tracing_messages.h (right): https://codereview.chromium.org/1042723002/diff/40001/components/tracing/tracing_messages.h#newcode32 components/tracing/tracing_messages.h:32: static_cast<int>(base::trace_event::MemoryDumpType::LAST)) On 2015/04/01 11:05:41, Primiano Tucci wrote: ...
5 years, 8 months ago (2015-04-01 15:55:36 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1042723002/60001
5 years, 8 months ago (2015-04-01 17:15:54 UTC) #15
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 8 months ago (2015-04-01 17:58:43 UTC) #16
commit-bot: I haz the power
5 years, 8 months ago (2015-04-01 17:59:56 UTC) #17
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/04f9de655915bfaf9fa2c90ee4848452ca71d89f
Cr-Commit-Position: refs/heads/master@{#323284}

Powered by Google App Engine
This is Rietveld 408576698