Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase

Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase. A few tests in FirstMeaningfulPaintDetectorTest break the DCHECK. Need to reset the initial value of their mock time function to be timeOrigin that was set in PerformanceBase.

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2622283007
Cr-Commit-Position: refs/heads/master@{#444134}
Committed: https://chromium.googlesource.com/chromium/src/+/eeda089e4c29b7ed01c16c7e95987b28cf8c5620

[sunjian, 2017-01-12 21:58:27]

Description was changed from

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase

BUG=680623
==========

to

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[sunjian, 2017-01-12 22:00:08]

sunjian@chromium.org changed reviewers:
+ panicker@chromium.org

[panicker, 2017-01-12 22:27:23]

Description was changed from

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[panicker, 2017-01-12 22:27:23]

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

[panicker, 2017-01-13 19:11:59]

https://codereview.chromium.org/2622283007/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
(right):

https://codereview.chromium.org/2622283007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp:51:
double timeOrigin() {
Update CL description mentioning why this file is updated.

https://codereview.chromium.org/2622283007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp:52:
Performance* performance =
DOMWindowPerformance::performance(*m_dummyPageHolder->frame().domWindow());
Nit: did you run git cl format?

[sunjian, 2017-01-13 19:48:45]

Description was changed from

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase. A few
tests in FirstMeaningfulPaintDetectorTest break the DCHECK. Need to reset the
initial value of their mock time function to be timeOrigin that was set in
PerformanceBase.

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[sunjian, 2017-01-13 19:49:34]

Addressed comments. Please take another look.

https://codereview.chromium.org/2622283007/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
(right):

https://codereview.chromium.org/2622283007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp:51:
double timeOrigin() {
On 2017/01/13 19:11:59, panicker wrote:
> Update CL description mentioning why this file is updated.

Done.

https://codereview.chromium.org/2622283007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp:52:
Performance* performance =
DOMWindowPerformance::performance(*m_dummyPageHolder->frame().domWindow());
On 2017/01/13 19:11:59, panicker wrote:
> Nit: did you run git cl format?

Done.

[panicker, 2017-01-13 22:00:15]

panicker@chromium.org changed reviewers:
+ skobes@chromium.org

[panicker, 2017-01-13 22:00:15]

LGTM.
+skobes for owners.

[skobes, 2017-01-13 22:31:44]

What's the motivation for this change?  The bug report has very little context.

Commit messages should be wrapped at a reasonable column boundary and begin with
a one-line summary (http://chris.beams.io/posts/git-commit/).

https://codereview.chromium.org/2622283007/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceBase.cpp (right):

https://codereview.chromium.org/2622283007/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceBase.cpp:572:
DOMHighResTimeStamp PerformanceBase::monotonicTimeToDOMHighResTimeStamp(
I suggest adding a comment to the declaration of this public function in the
header file, documenting the new precondition that monotonicTime > timeOrigin.

Have you audited the existing callers to ensure that none of them violate this?

[sunjian, 2017-01-14 00:59:40]

Patchset #4 (id:60001) has been deleted

[sunjian, 2017-01-14 01:04:32]

Addressed comments. Please take another look.

https://codereview.chromium.org/2622283007/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceBase.cpp (right):

https://codereview.chromium.org/2622283007/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceBase.cpp:572:
DOMHighResTimeStamp PerformanceBase::monotonicTimeToDOMHighResTimeStamp(
On 2017/01/13 22:31:44, skobes wrote:
> I suggest adding a comment to the declaration of this public function in the
> header file, documenting the new precondition that monotonicTime > timeOrigin.
> 
> Have you audited the existing callers to ensure that none of them violate
this?

Done. I don't think anyone else violates the DCHECK except for the tests we
modified in this patch.

[skobes, 2017-01-14 01:32:43]

lgtm

[sunjian, 2017-01-17 19:21:16]

The CQ bit was checked by sunjian@chromium.org

[sunjian, 2017-01-17 19:21:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
panicker@chromium.org
Link to the patchset: https://codereview.chromium.org/2622283007/#ps80001
(title: "added comments for monotonicTimeToDOMHighResTimeStamp in
PerformanceBase")

[commit-bot: I haz the power, 2017-01-17 19:21:55]

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-01-17 20:49:07]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484680876170200,
"parent_rev": "36c024e17a7c80294abbf2805d10635e2e3d9287", "commit_rev":
"eeda089e4c29b7ed01c16c7e95987b28cf8c5620"}

[commit-bot: I haz the power, 2017-01-17 20:49:37]

Description was changed from

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase. A few
tests in FirstMeaningfulPaintDetectorTest break the DCHECK. Need to reset the
initial value of their mock time function to be timeOrigin that was set in
PerformanceBase.

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase. A few
tests in FirstMeaningfulPaintDetectorTest break the DCHECK. Need to reset the
initial value of their mock time function to be timeOrigin that was set in
PerformanceBase.

BUG=680623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2017-01-17 20:49:38]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/eeda089e4c29b7ed01c16c7e9598...

[sunjian, 2017-01-25 21:17:57]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/2655123002/ by sunjian@chromium.org.

The reason for reverting is: * in chrome on android, go to google.com
* click on sign in at top left
* start typing in the input box for user name (I was trying to type
"webviewteam")
* crash

Fix: potentially get rid of the DCHECK and add an if statement to make sure the
output time is never negative..