Bug 193213 - Fix Resource Timing buffer edge cases for WPT
Summary: Fix Resource Timing buffer edge cases for WPT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-07 14:12 PST by cvazac
Modified: 2019-02-28 11:12 PST (History)
9 users (show)

See Also:


Attachments
Patch (2.29 KB, patch)
2019-01-07 14:15 PST, cvazac
no flags Details | Formatted Diff | Diff
Patch (2.63 KB, patch)
2019-01-07 14:17 PST, cvazac
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (2.61 MB, application/zip)
2019-01-07 15:20 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.00 MB, application/zip)
2019-01-07 15:33 PST, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-sierra (2.09 MB, application/zip)
2019-01-07 16:13 PST, Build Bot
no flags Details
Archive of layout-test-results from ews201 for win-future (12.83 MB, application/zip)
2019-01-07 16:36 PST, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (9.69 MB, application/zip)
2019-01-07 16:38 PST, Build Bot
no flags Details
Patch (33.93 KB, patch)
2019-01-26 13:07 PST, cvazac
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (2.43 MB, application/zip)
2019-01-26 14:10 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.60 MB, application/zip)
2019-01-26 14:24 PST, Build Bot
no flags Details
Archive of layout-test-results from ews200 for win-future (12.82 MB, application/zip)
2019-01-26 14:44 PST, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-highsierra (2.07 MB, application/zip)
2019-01-26 14:52 PST, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (9.14 MB, application/zip)
2019-01-26 15:07 PST, Build Bot
no flags Details
Patch (37.17 KB, patch)
2019-01-26 20:20 PST, cvazac
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.47 MB, application/zip)
2019-01-26 22:17 PST, Build Bot
no flags Details
Patch (37.17 KB, patch)
2019-01-28 08:21 PST, cvazac
no flags Details | Formatted Diff | Diff
Patch (36.42 KB, patch)
2019-01-28 08:23 PST, cvazac
no flags Details | Formatted Diff | Diff
Patch (6.42 KB, patch)
2019-02-14 07:15 PST, cvazac
no flags Details | Formatted Diff | Diff
Patch (7.14 KB, patch)
2019-02-14 07:34 PST, cvazac
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (2.42 MB, application/zip)
2019-02-14 08:37 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.70 MB, application/zip)
2019-02-14 08:55 PST, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-highsierra (2.04 MB, application/zip)
2019-02-14 09:18 PST, Build Bot
no flags Details
Archive of layout-test-results from ews201 for win-future (12.85 MB, application/zip)
2019-02-14 09:20 PST, Build Bot
no flags Details
Patch (8.53 KB, patch)
2019-02-14 09:33 PST, cvazac
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-highsierra (2.44 MB, application/zip)
2019-02-14 10:37 PST, Build Bot
no flags Details
Patch (10.17 KB, patch)
2019-02-14 10:55 PST, cvazac
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (9.59 MB, application/zip)
2019-02-14 14:16 PST, Build Bot
no flags Details
Patch (9.43 KB, patch)
2019-02-14 20:44 PST, cvazac
no flags Details | Formatted Diff | Diff
Patch (9.43 KB, patch)
2019-02-15 08:22 PST, cvazac
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description cvazac 2019-01-07 14:12:02 PST
Fix Resource Timing buffer edge cases for WPT
Comment 1 cvazac 2019-01-07 14:15:30 PST
Created attachment 358530 [details]
Patch
Comment 2 cvazac 2019-01-07 14:17:49 PST
Created attachment 358532 [details]
Patch
Comment 4 Build Bot 2019-01-07 15:20:05 PST
Comment on attachment 358532 [details]
Patch

Attachment 358532 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10663037

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 5 Build Bot 2019-01-07 15:20:07 PST
Created attachment 358540 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 6 Build Bot 2019-01-07 15:33:10 PST
Comment on attachment 358532 [details]
Patch

Attachment 358532 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10663138

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 7 Build Bot 2019-01-07 15:33:12 PST
Created attachment 358544 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 8 Build Bot 2019-01-07 16:13:57 PST
Comment on attachment 358532 [details]
Patch

Attachment 358532 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10663410

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 9 Build Bot 2019-01-07 16:13:59 PST
Created attachment 358552 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 10 Build Bot 2019-01-07 16:36:31 PST
Comment on attachment 358532 [details]
Patch

Attachment 358532 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10664196

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 11 Build Bot 2019-01-07 16:36:42 PST
Created attachment 358554 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 12 Build Bot 2019-01-07 16:38:00 PST
Comment on attachment 358532 [details]
Patch

Attachment 358532 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10663761

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 13 Build Bot 2019-01-07 16:38:02 PST
Created attachment 358555 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 14 youenn fablet 2019-01-08 09:04:57 PST
Comment on attachment 358532 [details]
Patch

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

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=193213

It seems one WebKit test needs rebasing.

> Source/WebCore/ChangeLog:13
> +        buffer-full-when-populate-entries.html

Should we import them in WebKit (you can do Tools/Scripts/import-w3c-tests -t web-platform-tests/resource-timing for that purpose)?

> Source/WebCore/ChangeLog:18
> +        (WebCore::Performance::resourceTimingBufferFullTimerFired):

Could you describe a little bit the changes?
Comment 15 cvazac 2019-01-26 13:07:23 PST
Created attachment 360247 [details]
Patch
Comment 16 Build Bot 2019-01-26 14:10:28 PST
Comment on attachment 360247 [details]
Patch

Attachment 360247 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10904194

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 17 Build Bot 2019-01-26 14:10:30 PST
Created attachment 360249 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 18 Build Bot 2019-01-26 14:24:00 PST
Comment on attachment 360247 [details]
Patch

Attachment 360247 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10904210

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 19 Build Bot 2019-01-26 14:24:02 PST
Created attachment 360250 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 20 Build Bot 2019-01-26 14:44:06 PST
Comment on attachment 360247 [details]
Patch

Attachment 360247 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10904292

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 21 Build Bot 2019-01-26 14:44:17 PST
Created attachment 360253 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 22 Build Bot 2019-01-26 14:52:54 PST
Comment on attachment 360247 [details]
Patch

Attachment 360247 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10904244

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 23 Build Bot 2019-01-26 14:52:56 PST
Created attachment 360254 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 24 Build Bot 2019-01-26 15:07:51 PST
Comment on attachment 360247 [details]
Patch

Attachment 360247 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10904275

New failing tests:
imported/w3c/web-platform-tests/resource-timing/buffer-full-add-after-full-event.html
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 25 Build Bot 2019-01-26 15:07:53 PST
Created attachment 360255 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 26 cvazac 2019-01-26 20:20:59 PST
Created attachment 360264 [details]
Patch
Comment 27 Build Bot 2019-01-26 22:17:40 PST
Comment on attachment 360264 [details]
Patch

Attachment 360264 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10907141

New failing tests:
imported/w3c/web-platform-tests/resource-timing/buffer-full-add-after-full-event.html
Comment 28 Build Bot 2019-01-26 22:17:42 PST
Created attachment 360268 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 29 cvazac 2019-01-28 08:21:11 PST
Created attachment 360344 [details]
Patch
Comment 30 cvazac 2019-01-28 08:23:43 PST
Created attachment 360345 [details]
Patch
Comment 31 youenn fablet 2019-01-28 10:15:55 PST
Comment on attachment 360345 [details]
Patch

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

> Source/WebCore/page/Performance.cpp:249
> +        if (beforeCount <= afterCount) {

If this check is true, it seems we would be loosing some information, how big an issue is it?
Should we do a smarter check here?
Something like, continue if we ever queue any entry. Or is there a risk with that approach?

> LayoutTests/imported/w3c/web-platform-tests/resource-timing/redirects.sub-expected.txt:7
> +FAIL Testing resource entries assert_unreached: http://localhost:8800/common/redirect.py?location=https://www.localhost:9443/resource-timing/resources/blank_page_green.htm?id=xhr is expected to be in the Resource Timing buffer Reached unreachable code

We probably could rewrite the test (as a WPT PR) to make www.localhost be 127.0.0.1 in that context.
Ditto below.
Comment 32 Yoav Weiss 2019-01-31 06:54:03 PST
Comment on attachment 360345 [details]
Patch

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

>> Source/WebCore/page/Performance.cpp:249
>> +        if (beforeCount <= afterCount) {
> 
> If this check is true, it seems we would be loosing some information, how big an issue is it?
> Should we do a smarter check here?
> Something like, continue if we ever queue any entry. Or is there a risk with that approach?

In the spec discussions, it was agreed (with rniwa) that this is the desired behaviour. If the event fired, but its handler did not increase the buffer size or cleared the entries, running the event again (because there are entries in the secondary buffer) is likely to result in an infinite loop.
So, in this rare case when there's a handler which doesn't make any room in the buffer, we will lose entries. That seems reasonable.
Comment 33 Ryosuke Niwa 2019-01-31 17:08:39 PST
Yeah, in this particular case, keep firing the event is probably no that useful. In particular, we don't want to keep the buffer full in the event the page wasn't listening to the event at all.
Comment 34 cvazac 2019-02-14 07:15:40 PST
Created attachment 362013 [details]
Patch
Comment 35 cvazac 2019-02-14 07:34:18 PST
Created attachment 362014 [details]
Patch
Comment 36 Build Bot 2019-02-14 08:37:24 PST
Comment on attachment 362014 [details]
Patch

Attachment 362014 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11147120

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 37 Build Bot 2019-02-14 08:37:26 PST
Created attachment 362020 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 38 Build Bot 2019-02-14 08:55:17 PST
Comment on attachment 362014 [details]
Patch

Attachment 362014 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11147143

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 39 Build Bot 2019-02-14 08:55:19 PST
Created attachment 362021 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 40 Build Bot 2019-02-14 09:18:51 PST
Comment on attachment 362014 [details]
Patch

Attachment 362014 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11147189

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 41 Build Bot 2019-02-14 09:18:53 PST
Created attachment 362024 [details]
Archive of layout-test-results from ews113 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 42 Build Bot 2019-02-14 09:20:36 PST
Comment on attachment 362014 [details]
Patch

Attachment 362014 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11147464

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 43 Build Bot 2019-02-14 09:20:48 PST
Created attachment 362025 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 44 cvazac 2019-02-14 09:33:59 PST
Created attachment 362028 [details]
Patch
Comment 45 Build Bot 2019-02-14 10:37:13 PST
Comment on attachment 362028 [details]
Patch

Attachment 362028 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11148272

New failing tests:
http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Comment 46 Build Bot 2019-02-14 10:37:15 PST
Created attachment 362034 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 47 cvazac 2019-02-14 10:55:48 PST
Created attachment 362039 [details]
Patch
Comment 48 Build Bot 2019-02-14 14:16:55 PST
Comment on attachment 362039 [details]
Patch

Attachment 362039 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11150028

New failing tests:
imported/w3c/web-platform-tests/resource-timing/buffer-full-add-after-full-event.html
Comment 49 Build Bot 2019-02-14 14:16:57 PST
Created attachment 362060 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 50 cvazac 2019-02-14 20:44:01 PST
Created attachment 362095 [details]
Patch
Comment 51 cvazac 2019-02-15 08:22:52 PST
Created attachment 362115 [details]
Patch
Comment 52 cvazac 2019-02-28 06:48:10 PST
@youennf can you PTAL? :)
Comment 53 WebKit Commit Bot 2019-02-28 11:09:38 PST
Comment on attachment 362115 [details]
Patch

Clearing flags on attachment: 362115

Committed r242209: <https://trac.webkit.org/changeset/242209>
Comment 54 WebKit Commit Bot 2019-02-28 11:09:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 55 Radar WebKit Bug Importer 2019-02-28 11:12:50 PST
<rdar://problem/48481194>