Add chrome.memoryBenchmarking.requestMemoryDump() API

Add chrome.memoryBenchmarking.requestMemoryDump() API

This patch adds a method for requesting memory dumps exposed by the
Memory Benchmarking extension:

  chrome.memoryBenchmarking.requestMemoryDump(
      function(dump_guid, success) {
    // Optional callback.
  });

The existing extension can be enabled by passing the
--enable-memory-benchmarking command-line flag to Chrome.

Note that this patch is a replacement for
https://codereview.chromium.org/1232313004/ and
https://codereview.chromium.org/1235393005/.

BUG=505826

[petrcermak, 2015-07-23 19:03:25]

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

[Primiano Tucci (use gerrit), 2015-07-24 14:29:13]

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

[Primiano Tucci (use gerrit), 2015-07-24 14:29:14]

makes sense to me, but at the same time I never dealt with gin before :).
LGTM with few small nits but please get a review also from somebody who speaks
v8/gin.
I think +jochen is a good pick here, as conveniently he is also an owner :)

https://codereview.chromium.org/1245233004/diff/1/content/renderer/memory_ben...
File content/renderer/memory_benchmarking_extension.cc (right):

https://codereview.chromium.org/1245233004/diff/1/content/renderer/memory_ben...
content/renderer/memory_benchmarking_extension.cc:43:
base::SingleThreadTaskRunner* task_runner() { return task_runner_.get(); }
nit +const between ) and {

https://codereview.chromium.org/1245233004/diff/1/content/renderer/memory_ben...
content/renderer/memory_benchmarking_extension.cc:45: v8::Isolate* isolate() {
return isolate_; }
ditto (here and below)

https://codereview.chromium.org/1245233004/diff/1/content/renderer/memory_ben...
content/renderer/memory_benchmarking_extension.cc:87: void
OnMemoryDumpFinished(scoped_ptr<CallbackWithContext> callback_with_context,
You don't need to split the function in OnMemoryDumpFinished + internal, are you
doing that just for the thread hopping?
Usually it is an accepted pattern in chrome to post to the function itself in
the right thread (without the extra layer of indirection).

[Primiano Tucci (use gerrit), 2015-07-24 14:30:02]

Ah also would be nice to have a liitle bit more context in the CL description.
Why is this needed? What problem does it help to solve? Add just a couple of
lines to explain why.

[pfeldman, 2015-07-24 14:55:54]

Does it work on mobile?

[pfeldman, 2015-07-24 15:04:26]

I guess it does when driver by telemetry. Not as straightforward as a ditect rdp
method - you need to wrap and process the js call on client, also not too nice
as it is driven by renderer for no good reason. Why not the rdp handler on the
browser side?

[pfeldman, 2015-07-24 15:13:45]

We had a great chat with Primiano where we seemed to agree that there are two
distinct use cases for dumps: dump on a timeline and a dump upon request. So we
should leave the Tracing.requestMemory for the first use case, factor memory
dump generator into the standalone API and reuse it from the nee Memory.* domain
for external use ouside the timeline that would do just memory upon request. All
these extensions and JS calls that drive things from the renderer and not the
great automation infrastruxture and I would not extend it. Rdp covers all the
content embedders, needs no flags and is processed directly on the browser.

[Primiano Tucci (use gerrit), 2015-07-24 15:26:11]

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

[Primiano Tucci (use gerrit), 2015-07-24 15:26:11]

+nduca and I had a chat and figured out that could have been easier and less
troublesome to prototype using a JS extension to iterate in the short term, and
once we figure out the right API surface we want between telemetry and chromium
we can move it to rdp. WDYT?

> Does it work on mobile?
I would imagine so. Petr, can you give a try?

>you need to wrap and process the js call on client
You are right, but we need some plumbing in telemetry in both cases.
https://codereview.chromium.org/1257693002/ is the telemetry counterpart to
this.

> also not too nice as it is driven by renderer for no good reason. Why not the
rdp handler on the browser side?
For the sake of memory-infra doesn't make too much difference. Regardless of
where requesting the dump from, the IPC storm happens anyways and the results
ends back in the right place.

[petrcermak, 2015-07-24 15:31:14]

On 2015/07/24 15:26:11, Primiano Tucci (OOO til Aug 9) wrote:
> +nduca and I had a chat and figured out that could have been easier and less
> troublesome to prototype using a JS extension to iterate in the short term,
and
> once we figure out the right API surface we want between telemetry and
chromium
> we can move it to rdp. WDYT?
> 
> > Does it work on mobile?
> I would imagine so. Petr, can you give a try?

I tested it and it works fine on both desktop and mobile (see
https://codereview.chromium.org/1257693002/).
 
> >you need to wrap and process the js call on client
> You are right, but we need some plumbing in telemetry in both cases.
> https://codereview.chromium.org/1257693002/ is the telemetry counterpart to
> this.
> 
> > also not too nice as it is driven by renderer for no good reason. Why not
the
> rdp handler on the browser side?
> For the sake of memory-infra doesn't make too much difference. Regardless of
> where requesting the dump from, the IPC storm happens anyways and the results
> ends back in the right place.

[nduca, 2015-07-24 20:56:02]

Pavel and I had a nice chat. It sounds like he's okay with us going the
tracing_handler route. So, lets go back to that approach, and then go from
there.

[jochen (gone - plz use gerrit), 2015-07-27 08:49:25]

lgtm

[petrcermak, 2015-07-29 16:18:47]

After some discussions we decided to go back to the original DevTools API
solution (https://codereview.chromium.org/1255213003,
https://codereview.chromium.org/1235393005/,
https://codereview.chromium.org/1232313004/), so this patch was abandoned.