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

commit ab76c201c1642d45a56af5bf550d84e575ffcea6
Author: Misha Efimov <mef@chromium.org>
Date: Tue Apr 10 15:46:45 2018

[Cronet] Native API should properly handle request cancellation.

Bug:  812334 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I99f120d624916af57ccbc068e5162b920299fc89
Reviewed-on: https://chromium-review.googlesource.com/984114
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549543}
[modify] https://crrev.com/ab76c201c1642d45a56af5bf550d84e575ffcea6/components/cronet/native/test/test_url_request_callback.cc
[modify] https://crrev.com/ab76c201c1642d45a56af5bf550d84e575ffcea6/components/cronet/native/test/test_url_request_callback.h
[modify] https://crrev.com/ab76c201c1642d45a56af5bf550d84e575ffcea6/components/cronet/native/test/url_request_test.cc
[modify] https://crrev.com/ab76c201c1642d45a56af5bf550d84e575ffcea6/components/cronet/native/url_request.cc
[modify] https://crrev.com/ab76c201c1642d45a56af5bf550d84e575ffcea6/components/cronet/native/url_request.h

I think this is fixed; can we mark it as so?

#1 is fixed.
#2 is not, though it's unclear whose responsibility it is to free things that are posted to but not executed from an executor.  It may be worth looking for C++17 or abseil message loop impls and how they handle this.  In Java these things are reference counted, and in Chromium I think they similarly get their references removed when the message loop is destroyed.

abseil doesn't have a message-loop.
Looks like C++ has some proposals but nothing final yet:
http://open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0443r1.html
(I don't understand much of this spec)

I get the feeling most message loop impls do take ownership of tasks and free tasks when being freed.  Some examples:
1. https://cs.chromium.org/chromium/src/base/message_loop/message_loop_task_runner.cc?rcl=27671e78306581405847bc386702cf3e0e239c25&l=128 notice how task is passed via std::move
2. https://arobenko.gitbooks.io/bare_metal_cpp/content/basic_concepts/event_loop.html  notice "taskPtr->~Task();" after executing task.
3. https://www.codeproject.com/Articles/1169105/Cplusplus-std-thread-Event-Loop-with-Message-Queue notice "delete userData; delete msg;" after executing task

Considering we also document executors to take ownership:
   * Takes ownership of |command| and runs it synchronously or asynchronously.
   * Destroys the |command| after execution.
I think it should be the responsibility of executors to destroy commands on shutdown too.

Seems like we need to add an OnDestroy() to Runnable.

Prototyped a fix at https://chromium-review.googlesource.com/c/chromium/src/+/1188846

The requirement that users call Cronet_Runnable_OnDestroy() and Cronet_Runnable_Destroy() seems too dangerous/prone to forgetting.  The idea is to just require _Destroy() calls and add test that users are actually doing this.
New idea:
1. make Runnable [Final] (where Final means users cannot create it, so they don't need to hear about and call OnDestroy)
2. remove public OnDestroy
3. if DCHECKs enabled, add test that user is calling Cronet_Runnable_Destroy(), log error message after 5s saying we're still waiting for Destroy() to be called.

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

commit a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3
Author: Paul Jensen <pauljensen@chromium.org>
Date: Wed Aug 29 19:43:19 2018

[Cronet] Make executors destroy runnables

Previously runnables would destroy themselves after running.
This caused leaks when executors were shutdown with unrun
runnables because they wouldn't get destroyed. With this
change executors are responsible for destroying runnables,
whether they are run or not. For internal runnables (the
ones that inherit from Cronet_Runnable) the destroy call
properly invokes virtual destructors.

This is the first of three CLs to fix runnable destruction,
the other two being:
1. when DCHECKs are enabled verify that embedder's executors
   do in fact destroy the runnables.
2. disallow user created runnables as they have no way of
   catching destruction and properly cleaning up.

Bug:  812334 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ifcdb96bd7991b9a5757ff65b150ffd2a3d5f73fe
Reviewed-on: https://chromium-review.googlesource.com/1193966
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587252}
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/cronet.idl
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/runnables.cc
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/runnables_unittest.cc
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/test/buffer_test.cc
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/test/executors_test.cc
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/test/test_upload_data_provider.cc
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/test/test_url_request_callback.cc
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/test/test_util.cc
[modify] https://crrev.com/a7be2aecc61ac52a3bc19d3acb9e3ae38599aae3/components/cronet/native/test/test_util.h

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

commit ed731e896460282772340ec53cb7f7361f151ad7
Author: Paul Jensen <pauljensen@chromium.org>
Date: Thu Sep 13 16:35:47 2018

[Cronet] Enforce that Executors call Cronet_Runnable_Destroy()

When DCHECKs are enabled, enforce that the Executors that apps
implement and use for Cronet_UrlRequests call Cronet_Runnable_Run()
and then Cronet_Runnable_Destroy().  If Executor doesn't call
Run() and Destroy() after 5s, post log message.  This serves to
enforce that apps don't leak Runnables.

Bug:  812334 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
Change-Id: I9f84c38d3b2ab41a7558037ae6e3852e196d08db
Reviewed-on: https://chromium-review.googlesource.com/1204430
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591039}
[modify] https://crrev.com/ed731e896460282772340ec53cb7f7361f151ad7/components/cronet/native/cronet.idl
[modify] https://crrev.com/ed731e896460282772340ec53cb7f7361f151ad7/components/cronet/native/url_request.cc
[modify] https://crrev.com/ed731e896460282772340ec53cb7f7361f151ad7/components/cronet/native/url_request.h