New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 680623 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Add DCHECK to monotonicTimeToDOMHighResTimeStamp in PerformanceBase

Project Member Reported by sunjian@chromium.org, Jan 12 2017

Issue description

Add DCHECK to monotonicTimeToDOMHighResTimeStamp in PerformanceBase. It doesn't make much sense for DOMHighResTimeStamp monotonicTimeToDOMHighResTimeStamp to produce a negative result.  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.
 
Description: Show this description
Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 17 2017

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

commit eeda089e4c29b7ed01c16c7e95987b28cf8c5620
Author: sunjian <sunjian@chromium.org>
Date: Tue Jan 17 20:49:02 2017

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}

[modify] https://crrev.com/eeda089e4c29b7ed01c16c7e95987b28cf8c5620/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
[modify] https://crrev.com/eeda089e4c29b7ed01c16c7e95987b28cf8c5620/third_party/WebKit/Source/core/timing/PerformanceBase.cpp
[modify] https://crrev.com/eeda089e4c29b7ed01c16c7e95987b28cf8c5620/third_party/WebKit/Source/core/timing/PerformanceBase.h

Status: Fixed (was: Untriaged)

Comment 5 by boliu@chromium.org, Jan 25 2017

Cc: boliu@chromium.org
Labels: -Pri-3 Pri-1
Status: Assigned (was: Fixed)
Hitting DCHECK added in that CL following these steps:

* 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

[FATAL:PerformanceBase.cpp(583)] Check failed: timeInSeconds >= 0 (-1380.97 vs. 0)

Stack Trace:
  RELADDR   FUNCTION                            FILE:LINE
  0009fb5f  ~LogMessage                         /android/chromium/src/base/logging.cc:537
  00c7880f  monotonicTimeToDOMHighResTimeStamp  /android/chromium/src/third_party/WebKit/Source/core/timing/PerformanceBase.cpp:583
  007e6949  timeStamp                           /android/chromium/src/third_party/WebKit/Source/core/events/Event.cpp:333
  v------>  timeStampAttributeGetter            /android/chromium/src/out/Default/gen/blink/bindings/core/v8/V8Event.cpp:177
  00d69353  timeStampAttributeGetterCallback    /android/chromium/src/out/Default/gen/blink/bindings/core/v8/V8Event.cpp:181

-----------------------------------------------------

     r0 00000000  r1 0000235e  r2 00000006  r3 9a504978
     r4 9a504980  r5 9a504930  r6 00000002  r7 0000010c
     r8 9a501404  r9 00000000  sl 960a8337  fp 9a501948
     ip 00000006  sp 9a501398  lr b6c8db61  pc b6c8ff50

Stack Trace:
  RELADDR   FUNCTION                            FILE:LINE
  00041f50  tgkill+12                           /system/lib/libc.so
  0003fb5d  pthread_kill+32                     /system/lib/libc.so
  0001c30f  raise+10                            /system/lib/libc.so
  000194c1  __libc_android_abort+34             /system/lib/libc.so
  000174ac  abort+4                             /system/lib/libc.so
  v------>  DebugBreak                          /android/chromium/src/base/debug/debugger_posix.cc:221
  00088b9d  BreakDebugger                       /android/chromium/src/base/debug/debugger_posix.cc:251
  0009fdef  ~LogMessage                         /android/chromium/src/base/logging.cc:759
  00c7880f  monotonicTimeToDOMHighResTimeStamp  /android/chromium/src/third_party/WebKit/Source/core/timing/PerformanceBase.cpp:583
  007e6947  timeStamp                           /android/chromium/src/third_party/WebKit/Source/core/events/Event.cpp:333
  v------>  timeStampAttributeGetter            /android/chromium/src/out/Default/gen/blink/bindings/core/v8/V8Event.cpp:177
  00d69353  timeStampAttributeGetterCallback    /android/chromium/src/out/Default/gen/blink/bindings/core/v8/V8Event.cpp:181
  000e98df  Call                                /android/chromium/src/v8/src/api-arguments.cc:25
  00182705  HandleApiCallHelper<false>          /android/chromium/src/v8/src/builtins/builtins-api.cc:106
  00181b41  InvokeApiFunction                   /android/chromium/src/v8/src/builtins/builtins-api.cc:211
  00552a31  GetPropertyWithAccessor             /android/chromium/src/v8/src/objects.cc:1437
  00552189  GetProperty                         /android/chromium/src/v8/src/objects.cc:1062
  004c9709  Load                                /android/chromium/src/v8/src/ic/ic.cc:695
  004cea27  Load                                /android/chromium/src/v8/src/ic/ic.cc:1675
  004d4c3b  __RT_impl_Runtime_KeyedLoadIC_Miss  /android/chromium/src/v8/src/ic/ic.cc:2660
We are gonna roll it back for now.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 26 2017

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

commit 6468f57d306b277ce67c2ed44e5eff4caa8d7286
Author: sunjian <sunjian@chromium.org>
Date: Thu Jan 26 07:24:54 2017

Revert of Added DCHECK for monotonicTimeToDOMHighResTimeStamp in PerformanceBase (patchset #4 id:80001 of https://codereview.chromium.org/2622283007/ )

Reason for revert:
* 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.

Original issue's description:
> 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

TBR=panicker@chromium.org,ksakamoto@chromium.org,skobes@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 680623 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2655123002
Cr-Commit-Position: refs/heads/master@{#446267}

[modify] https://crrev.com/6468f57d306b277ce67c2ed44e5eff4caa8d7286/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
[modify] https://crrev.com/6468f57d306b277ce67c2ed44e5eff4caa8d7286/third_party/WebKit/Source/core/timing/PerformanceBase.cpp
[modify] https://crrev.com/6468f57d306b277ce67c2ed44e5eff4caa8d7286/third_party/WebKit/Source/core/timing/PerformanceBase.h

Status: Fixed (was: Assigned)

Sign in to add a comment