ResourceLoadTiming should not be assumed to be available in ResourceTiming.

ResourceLoadTiming should not be assumed to be available in ResourceTiming.
fetchStart() was making such an assumption and dereferencing null causing crashes in prod.

BUG=560564
Review-Url: https://codereview.chromium.org/2741563002
Cr-Commit-Position: refs/heads/master@{#455643}
Committed: https://chromium.googlesource.com/chromium/src/+/c59fcd06cbe444483d1a53c30ae4dc44544957ff

[panicker, 2017-03-08 23:16:51]

https://codereview.chromium.org/2741563002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceResourceTiming.cpp (left):

https://codereview.chromium.org/2741563002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceResourceTiming.cpp:141: //
AppCache is exposed from chrome network stack, crbug/251100
This assumption was apparently fine in Jun 2013 (in this patch:
https://chromiumcodereview.appspot.com/16950017/) when the code was authored,
but has since become invalid. The crashes started in Nov 2015.
All other dereferences of m_timing check for null, except for this one.

[panicker, 2017-03-08 23:21:01]

panicker@chromium.org changed reviewers:
+ japhet@chromium.org, ksakamoto@chromium.org

[panicker, 2017-03-08 23:21:01]

Proposed fix for long standing bug: crashes reported in https://crbug.com/560564

[Nate Chapin, 2017-03-08 23:29:26]

I assume we don't have a repro? LGTM, I guess. I don't like converting asserts
to early-exits without a repro in general, but it's consistent with the rest of
the class, so I'll get over it.

[panicker, 2017-03-08 23:52:33]

On 2017/03/08 23:29:26, Nate Chapin wrote:
> I assume we don't have a repro? LGTM, I guess. I don't like converting asserts
> to early-exits without a repro in general, but it's consistent with the rest
of
> the class, so I'll get over it.

Thanks Nate.
I've tried repro-ing but no luck. Tried URLs here:
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Android%27%20...

[Kunihiko Sakamoto, 2017-03-09 00:00:00]

lgtm

[panicker, 2017-03-09 00:10:01]

The CQ bit was checked by panicker@chromium.org

[commit-bot: I haz the power, 2017-03-09 00:11:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 01:02:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 01:02:13]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[panicker, 2017-03-09 01:37:18]

The CQ bit was checked by panicker@chromium.org

[commit-bot: I haz the power, 2017-03-09 01:37:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 02:23:35]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489023438927880, "parent_rev":
"a2d48d4e791055524b1dd7870ed2674c98cc6bfc", "commit_rev":
"c59fcd06cbe444483d1a53c30ae4dc44544957ff"}

[commit-bot: I haz the power, 2017-03-09 02:24:16]

Description was changed from

==========
ResourceLoadTiming should not be assumed to be available in ResourceTiming.
fetchStart() was making such an assumption and dereferencing null causing
crashes in prod.

BUG=560564
==========

to

==========
ResourceLoadTiming should not be assumed to be available in ResourceTiming.
fetchStart() was making such an assumption and dereferencing null causing
crashes in prod.

BUG=560564

Review-Url: https://codereview.chromium.org/2741563002
Cr-Commit-Position: refs/heads/master@{#455643}
Committed:
https://chromium.googlesource.com/chromium/src/+/c59fcd06cbe444483d1a53c30ae4...
==========

[commit-bot: I haz the power, 2017-03-09 02:24:18]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/c59fcd06cbe444483d1a53c30ae4...