Bug 157897 - Web Inspector: timelines should not update via requestAnimationFrame unless Web Inspector is visible
Summary: Web Inspector: timelines should not update via requestAnimationFrame unless W...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: DoNotImportToRadar, InRadar
Depends on:
Blocks: 171034
  Show dependency treegraph
 
Reported: 2016-05-19 09:04 PDT by BJ Burg
Modified: 2017-04-23 20:15 PDT (History)
8 users (show)

See Also:


Attachments
Proposed Fix (14.67 KB, patch)
2016-05-19 09:21 PDT, BJ Burg
timothy: review+
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (1.40 MB, application/zip)
2016-05-19 10:18 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2016-05-19 09:04:00 PDT
.
Comment 1 BJ Burg 2016-05-19 09:04:27 PDT
<rdar://problem/26330802>
Comment 2 BJ Burg 2016-05-19 09:21:48 PDT
Created attachment 279393 [details]
Proposed Fix
Comment 3 Build Bot 2016-05-19 10:18:04 PDT
Comment on attachment 279393 [details]
Proposed Fix

Attachment 279393 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1348904

Number of test failures exceeded the failure limit.
Comment 4 Build Bot 2016-05-19 10:18:07 PDT
Created attachment 279399 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 BJ Burg 2016-05-19 10:57:17 PDT
Committed r201171: <http://trac.webkit.org/changeset/201171>
Comment 6 Joseph Pecoraro 2016-10-17 20:47:12 PDT
Comment on attachment 279393 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=279393&action=review

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:443
> +        if (typeof startTime === "number" && !isNaN(this._currentTime))
>              this._currentTime = startTime;
> -        else if (!isNaN(this._currentTime)) {
> +        else {

I don't understand this logic change. It ends up breaking the backend informing the frontend what the current time is in the common case (a user inspecting a local page). I put up a patch to revert this in bug 163583. If you think this is still needed for some reason, please comment there. I believe this patch was meant for Safari WebDriver --inspect behavior, but I don't understand why we wouldn't set the current time here if we are visible and have a start time.