Adding instrumentation to locate the source of jankiness.

Adding instrumentation to locate the source of jankiness.

Please see the bug for the context.
Prior instrumentations showed:

The jank happens in more than 1 place:

URLRequest::Delegate::OnResponseStarted   34.3 jph
NetworkDelegate::OnRawBytesRead    16.4 jph
URLRequest::Delegate::OnReadCompleted   7.62 jph
... (the list continues, but further jankiness numbers are quite low)

I need to instrument
the code inside it to find out which part causes jank.

This is a mechanical change that adds instrumentation required to locate the
source of jankiness (i.e. a long-running fragment of code executed as a part of
the task that causes jank) in the code. See the bug for details on what kind of
jank we are after.
A number of similar CLs were landed, and none of them caused issues. They've
helped to find and fix janky code. The code of the instrumentation is highly
optimized and is not expected to affect performance. The code simply creates a
diagnostic task which is identical to ones created by PostTask or IPC message
handlers. The task gets created only in developer build and in Canary channel.

BUG=423948
TBR=jianli,danakj,jhawkins

Committed: https://crrev.com/81ef0df2b58b26110066d43ea6a55f9f23926aea
Cr-Commit-Position: refs/heads/master@{#304653}

[vadimt, 2014-11-14 16:47:31]

vadimt@chromium.org changed reviewers:
+ danakj@chromium.org, jamiewalch@chromium.org, jhawkins@chromium.org,
jianli@chromium.org, michaeln@chromium.org, mmenke@chromium.org,
nick@chromium.org, phajdan.jr@chromium.org, vitalybuka@chromium.org

[vadimt, 2014-11-14 16:47:32]

mmenke, please review chrome/browser/net, content/browser/loader and net/
vitalybuka, cloud print
jamiewalch, remoting
danakj, storage/
nick, task_manager
phajdan.jr, resource_throttle
jhawkins, resource prefetcher
jianli, mock_url_request_delegate
michaeln:
 content/browser/appcache/appcache_update_job.cc
 content/browser/service_worker/service_worker_cache.cc
 content/browser/service_worker/service_worker_url_request_job.cc
 content/browser/service_worker/service_worker_write_to_cache_job.cc

[Jamie, 2014-11-14 18:26:45]

Nothing in remoting/ gets compiled into Chrome, so I don't think you need the
changes there.

[vadimt, 2014-11-14 19:12:20]

Done

[vadimt, 2014-11-14 19:12:41]

vadimt@chromium.org changed reviewers:
- jamiewalch@chromium.org

[Vitaly Buka (NO REVIEWS), 2014-11-14 22:18:51]

cloud print lgtm

[Vitaly Buka (NO REVIEWS), 2014-11-14 22:24:02]

On 2014/11/14 22:18:51, Vitaly Buka wrote:
> cloud print lgtm

Actually cloud print change is not helpful.
Nothing from cloud_print/ dir is build into Chrome browser.

[vadimt, 2014-11-14 22:35:31]

Thanks; reverted the file.

[vadimt, 2014-11-14 22:36:01]

vadimt@chromium.org changed reviewers:
- vitalybuka@chromium.org

[michaeln, 2014-11-15 00:52:21]

browser/appcache + browser/service_worker lgtm

[Paweł Hajdan Jr., 2014-11-17 10:36:58]

resource_throttle LGTM

[mmenke, 2014-11-17 15:38:16]

You've missed a whole bunch of throttles and ResourceHandlers.  Is this
deliberate?

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/net/conne...
File chrome/browser/net/connection_tester.cc (right):

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/net/conne...
chrome/browser/net/connection_tester.cc:366: "422516
ConnectionTester::TestRunner::OnReadCompleted"));
This file is not worth instrumenting.  It's only used from about:net-internals,
when manually triggered.  So it's not used often enough to be a common cause of
jank...And even if it were, we don't care.  That's basically internal-only API
intended for Chrome development / debugging (Not general web development).

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/task_mana...
File chrome/browser/task_manager/task_manager.cc (right):

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/task_mana...
chrome/browser/task_manager/task_manager.cc:1236: "423948
TaskManagerModel::NotifyBytesRead"));
I suggest adding another one just above the bytes_read_buffer_.push_back call. 
That seems the most likely culprit for OnRawBytesRead.

https://codereview.chromium.org/732633002/diff/40001/content/browser/loader/p...
File content/browser/loader/power_save_block_resource_throttle.cc (right):

https://codereview.chromium.org/732633002/diff/40001/content/browser/loader/p...
content/browser/loader/power_save_block_resource_throttle.cc:5: #include
"base/profiler/scoped_tracker.h"
This should go above the "content/public/browser/power_save_blocker.h" line

[ncarter (slow), 2014-11-18 01:08:24]

task_manager lgtm

[vadimt, 2014-11-18 01:14:27]

mmenke@:

Yes.
What you see is a combination of 2 kinds of changes:
1. Changes i'd do if I didn't ask for advice. This is basically instrumenting
internals of previous instrumentations that turned to be jankiest, but without
going too deep.
2. Instrumentations from recommendations (see internal email thread), where I
sometimes used freedom to apply recommendations selectively where otherwise I’d
have tons of instrumentations.

Now, I didn't instrument methods with empty implementations, and methods used
only by unit tests.

Perhaps, all the above is responsible for somewhat patchy result, but, yes,
everything is deliberate.

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/net/conne...
File chrome/browser/net/connection_tester.cc (right):

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/net/conne...
chrome/browser/net/connection_tester.cc:366: "422516
ConnectionTester::TestRunner::OnReadCompleted"));
On 2014/11/17 15:38:16, mmenke wrote:
> This file is not worth instrumenting.  It's only used from
about:net-internals,
> when manually triggered.  So it's not used often enough to be a common cause
of
> jank...And even if it were, we don't care.  That's basically internal-only API
> intended for Chrome development / debugging (Not general web development).

Done.

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/task_mana...
File chrome/browser/task_manager/task_manager.cc (right):

https://codereview.chromium.org/732633002/diff/40001/chrome/browser/task_mana...
chrome/browser/task_manager/task_manager.cc:1236: "423948
TaskManagerModel::NotifyBytesRead"));
On 2014/11/17 15:38:16, mmenke wrote:
> I suggest adding another one just above the bytes_read_buffer_.push_back call.

> That seems the most likely culprit for OnRawBytesRead.

Done.

https://codereview.chromium.org/732633002/diff/40001/content/browser/loader/p...
File content/browser/loader/power_save_block_resource_throttle.cc (right):

https://codereview.chromium.org/732633002/diff/40001/content/browser/loader/p...
content/browser/loader/power_save_block_resource_throttle.cc:5: #include
"base/profiler/scoped_tracker.h"
On 2014/11/17 15:38:16, mmenke wrote:
> This should go above the "content/public/browser/power_save_blocker.h" line

Done.

[mmenke, 2014-11-18 15:15:38]

LGTM

[vadimt, 2014-11-18 18:47:51]

The CQ bit was checked by vadimt@chromium.org

[commit-bot: I haz the power, 2014-11-18 18:48:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/732633002/60001

[commit-bot: I haz the power, 2014-11-18 19:42:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-11-18 19:44:19]

Patchset 4 (id:??) landed as
https://crrev.com/81ef0df2b58b26110066d43ea6a55f9f23926aea
Cr-Commit-Position: refs/heads/master@{#304653}