WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
195623
Link prefetch not useful for top-level navigation
https://bugs.webkit.org/show_bug.cgi?id=195623
Summary
Link prefetch not useful for top-level navigation
Rob Buis
Reported
2019-03-12 09:03:04 PDT
Due to cache partitioning this will be a cache miss and the fetch will be for nothing.
Attachments
Patch
(19.27 KB, patch)
2019-03-12 09:32 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-highsierra
(2.48 MB, application/zip)
2019-03-12 10:38 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews114 for mac-highsierra
(2.28 MB, application/zip)
2019-03-12 11:20 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews206 for win-future
(12.98 MB, application/zip)
2019-03-12 11:46 PDT
,
EWS Watchlist
no flags
Details
Patch
(15.12 KB, patch)
2019-03-14 14:31 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-highsierra
(2.67 MB, application/zip)
2019-03-14 15:28 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews105 for mac-highsierra-wk2
(2.74 MB, application/zip)
2019-03-14 15:43 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews116 for mac-highsierra
(2.40 MB, application/zip)
2019-03-14 16:21 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews201 for win-future
(12.89 MB, application/zip)
2019-03-14 17:20 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews122 for ios-simulator-wk2
(3.38 MB, application/zip)
2019-03-14 19:49 PDT
,
EWS Watchlist
no flags
Details
Patch
(26.42 KB, patch)
2019-03-18 08:57 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-highsierra
(2.45 MB, application/zip)
2019-03-18 10:07 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews105 for mac-highsierra-wk2
(2.70 MB, application/zip)
2019-03-18 10:17 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews114 for mac-highsierra
(2.30 MB, application/zip)
2019-03-18 10:56 PDT
,
EWS Watchlist
no flags
Details
Patch
(25.75 KB, patch)
2019-03-18 11:45 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-highsierra
(2.56 MB, application/zip)
2019-03-18 12:59 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews114 for mac-highsierra
(2.44 MB, application/zip)
2019-03-18 14:07 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews204 for win-future
(12.87 MB, application/zip)
2019-03-18 18:48 PDT
,
EWS Watchlist
no flags
Details
Patch
(23.86 KB, patch)
2019-03-19 09:40 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-highsierra
(2.67 MB, application/zip)
2019-03-19 10:42 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews201 for win-future
(12.86 MB, application/zip)
2019-03-19 11:51 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews112 for mac-highsierra
(2.26 MB, application/zip)
2019-03-19 11:54 PDT
,
EWS Watchlist
no flags
Details
Patch
(24.67 KB, patch)
2019-03-19 13:49 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-highsierra
(3.12 MB, application/zip)
2019-03-19 15:02 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2
(2.95 MB, application/zip)
2019-03-19 15:46 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews115 for mac-highsierra
(2.25 MB, application/zip)
2019-03-19 16:21 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews202 for win-future
(12.98 MB, application/zip)
2019-03-19 16:55 PDT
,
EWS Watchlist
no flags
Details
Patch
(25.38 KB, patch)
2019-03-20 04:05 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews200 for win-future
(12.85 MB, application/zip)
2019-03-20 05:47 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2
(2.86 MB, application/zip)
2019-03-20 06:19 PDT
,
EWS Watchlist
no flags
Details
Patch
(25.84 KB, patch)
2019-03-20 14:30 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews206 for win-future
(12.91 MB, application/zip)
2019-03-20 18:38 PDT
,
EWS Watchlist
no flags
Details
Patch
(28.44 KB, patch)
2019-03-21 08:40 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(27.84 KB, patch)
2019-03-21 10:06 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews200 for win-future
(12.87 MB, application/zip)
2019-03-21 12:36 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2
(2.72 MB, application/zip)
2019-03-21 17:13 PDT
,
EWS Watchlist
no flags
Details
Patch
(27.64 KB, patch)
2019-03-22 02:07 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews104 for mac-highsierra-wk2
(2.78 MB, application/zip)
2019-03-22 03:27 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews202 for win-future
(12.93 MB, application/zip)
2019-03-22 04:05 PDT
,
EWS Watchlist
no flags
Details
Patch
(28.88 KB, patch)
2019-03-25 09:44 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews206 for win-future
(12.88 MB, application/zip)
2019-03-25 12:15 PDT
,
EWS Watchlist
no flags
Details
Patch
(28.88 KB, patch)
2019-03-25 12:33 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(28.88 KB, patch)
2019-03-26 12:38 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(28.84 KB, patch)
2019-03-26 14:42 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2
(2.63 MB, application/zip)
2019-03-27 00:01 PDT
,
EWS Watchlist
no flags
Details
Patch
(31.20 KB, patch)
2019-03-27 07:39 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(21.14 KB, patch)
2019-03-29 04:56 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(32.48 KB, patch)
2019-03-29 08:37 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-highsierra
(2.46 MB, application/zip)
2019-03-29 09:45 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews204 for win-future
(12.87 MB, application/zip)
2019-03-29 10:25 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews114 for mac-highsierra
(2.27 MB, application/zip)
2019-03-29 10:35 PDT
,
EWS Watchlist
no flags
Details
Patch
(35.11 KB, patch)
2019-04-02 01:44 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-highsierra
(2.46 MB, application/zip)
2019-04-02 02:53 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews117 for mac-highsierra
(2.25 MB, application/zip)
2019-04-02 03:35 PDT
,
EWS Watchlist
no flags
Details
Patch
(35.28 KB, patch)
2019-04-02 05:11 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(35.80 KB, patch)
2019-04-03 01:39 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(33.35 KB, patch)
2019-04-03 01:59 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(33.41 KB, patch)
2019-04-03 07:38 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(33.06 KB, patch)
2019-04-13 05:07 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(32.96 KB, patch)
2019-04-13 08:17 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(33.05 KB, patch)
2019-04-14 12:55 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(1.85 KB, patch)
2019-04-16 06:49 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(33.17 KB, patch)
2019-04-19 09:46 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(31.48 KB, patch)
2019-04-20 10:12 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(31.43 KB, patch)
2019-04-22 02:59 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(32.88 KB, patch)
2019-04-22 23:48 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(32.11 KB, patch)
2019-04-24 08:00 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-highsierra
(3.07 MB, application/zip)
2019-04-24 09:10 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews113 for mac-highsierra
(2.99 MB, application/zip)
2019-04-24 09:52 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews123 for ios-simulator-wk2
(2.61 MB, application/zip)
2019-04-24 10:00 PDT
,
EWS Watchlist
no flags
Details
Patch
(32.48 KB, patch)
2019-04-24 10:09 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(32.08 KB, patch)
2019-04-26 01:02 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews123 for ios-simulator-wk2
(3.10 MB, application/zip)
2019-04-26 02:47 PDT
,
EWS Watchlist
no flags
Details
Patch
(32.08 KB, patch)
2019-04-26 08:14 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(31.95 KB, patch)
2019-04-26 09:56 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(31.72 KB, patch)
2019-04-26 12:33 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(31.77 KB, patch)
2019-05-08 04:10 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Show Obsolete
(76)
View All
Add attachment
proposed patch, testcase, etc.
Rob Buis
Comment 1
2019-03-12 09:32:05 PDT
Created
attachment 364392
[details]
Patch
EWS Watchlist
Comment 2
2019-03-12 10:38:50 PDT
Comment on
attachment 364392
[details]
Patch
Attachment 364392
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11476671
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 3
2019-03-12 10:38:52 PDT
Created
attachment 364402
[details]
Archive of layout-test-results from ews101 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 4
2019-03-12 11:20:42 PDT
Comment on
attachment 364392
[details]
Patch
Attachment 364392
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11476877
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 5
2019-03-12 11:20:44 PDT
Created
attachment 364415
[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
EWS Watchlist
Comment 6
2019-03-12 11:46:06 PDT
Comment on
attachment 364392
[details]
Patch
Attachment 364392
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11477510
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 7
2019-03-12 11:46:18 PDT
Created
attachment 364419
[details]
Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Rob Buis
Comment 8
2019-03-14 14:31:34 PDT
Created
attachment 364687
[details]
Patch
EWS Watchlist
Comment 9
2019-03-14 15:27:59 PDT
Comment on
attachment 364687
[details]
Patch
Attachment 364687
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11508255
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 10
2019-03-14 15:28:01 PDT
Created
attachment 364704
[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
EWS Watchlist
Comment 11
2019-03-14 15:43:22 PDT
Comment on
attachment 364687
[details]
Patch
Attachment 364687
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11508376
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 12
2019-03-14 15:43:24 PDT
Created
attachment 364708
[details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 13
2019-03-14 16:21:48 PDT
Comment on
attachment 364687
[details]
Patch
Attachment 364687
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11508667
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 14
2019-03-14 16:21:50 PDT
Created
attachment 364712
[details]
Archive of layout-test-results from ews116 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-highsierra Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 15
2019-03-14 17:20:34 PDT
Comment on
attachment 364687
[details]
Patch
Attachment 364687
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11509560
New failing tests: http/tests/cache/memory-cache-pruning.html http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 16
2019-03-14 17:20:48 PDT
Created
attachment 364725
[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
EWS Watchlist
Comment 17
2019-03-14 19:49:29 PDT
Comment on
attachment 364687
[details]
Patch
Attachment 364687
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/11511672
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 18
2019-03-14 19:49:31 PDT
Created
attachment 364761
[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
Rob Buis
Comment 19
2019-03-18 08:57:06 PDT
Created
attachment 365017
[details]
Patch
EWS Watchlist
Comment 20
2019-03-18 10:06:58 PDT
Comment on
attachment 365017
[details]
Patch
Attachment 365017
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11551328
New failing tests: http/tests/cache/display-image-unset-allows-cached-image-load.html http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 21
2019-03-18 10:07:00 PDT
Created
attachment 365023
[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
EWS Watchlist
Comment 22
2019-03-18 10:17:22 PDT
Comment on
attachment 365017
[details]
Patch
Attachment 365017
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11551341
New failing tests: http/tests/cache/display-image-unset-allows-cached-image-load.html
EWS Watchlist
Comment 23
2019-03-18 10:17:24 PDT
Created
attachment 365024
[details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 24
2019-03-18 10:56:44 PDT
Comment on
attachment 365017
[details]
Patch
Attachment 365017
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11551491
New failing tests: http/tests/cache/display-image-unset-allows-cached-image-load.html http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 25
2019-03-18 10:56:46 PDT
Created
attachment 365030
[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
Rob Buis
Comment 26
2019-03-18 11:45:33 PDT
Created
attachment 365040
[details]
Patch
EWS Watchlist
Comment 27
2019-03-18 12:59:20 PDT
Comment on
attachment 365040
[details]
Patch
Attachment 365040
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11553059
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 28
2019-03-18 12:59:22 PDT
Created
attachment 365045
[details]
Archive of layout-test-results from ews101 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 29
2019-03-18 14:07:22 PDT
Comment on
attachment 365040
[details]
Patch
Attachment 365040
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11553295
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 30
2019-03-18 14:07:24 PDT
Created
attachment 365058
[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
EWS Watchlist
Comment 31
2019-03-18 18:48:18 PDT
Comment on
attachment 365040
[details]
Patch
Attachment 365040
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11556810
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 32
2019-03-18 18:48:30 PDT
Created
attachment 365101
[details]
Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Rob Buis
Comment 33
2019-03-19 09:40:15 PDT
Created
attachment 365173
[details]
Patch
EWS Watchlist
Comment 34
2019-03-19 10:42:04 PDT
Comment on
attachment 365173
[details]
Patch
Attachment 365173
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11567417
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 35
2019-03-19 10:42:06 PDT
Created
attachment 365183
[details]
Archive of layout-test-results from ews103 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 36
2019-03-19 11:51:25 PDT
Comment on
attachment 365173
[details]
Patch
Attachment 365173
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11568089
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 37
2019-03-19 11:51:37 PDT
Created
attachment 365194
[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
EWS Watchlist
Comment 38
2019-03-19 11:54:54 PDT
Comment on
attachment 365173
[details]
Patch
Attachment 365173
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11567939
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 39
2019-03-19 11:54:56 PDT
Created
attachment 365198
[details]
Archive of layout-test-results from ews112 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Rob Buis
Comment 40
2019-03-19 13:49:54 PDT
Created
attachment 365223
[details]
Patch
EWS Watchlist
Comment 41
2019-03-19 15:02:18 PDT
Comment on
attachment 365223
[details]
Patch
Attachment 365223
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11570649
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 42
2019-03-19 15:02:21 PDT
Created
attachment 365235
[details]
Archive of layout-test-results from ews103 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 43
2019-03-19 15:46:32 PDT
Comment on
attachment 365223
[details]
Patch
Attachment 365223
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11571097
New failing tests: imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html
EWS Watchlist
Comment 44
2019-03-19 15:46:34 PDT
Created
attachment 365248
[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 45
2019-03-19 16:21:14 PDT
Comment on
attachment 365223
[details]
Patch
Attachment 365223
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11571187
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 46
2019-03-19 16:21:17 PDT
Created
attachment 365253
[details]
Archive of layout-test-results from ews115 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 47
2019-03-19 16:55:36 PDT
Comment on
attachment 365223
[details]
Patch
Attachment 365223
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11571929
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 48
2019-03-19 16:55:48 PDT
Created
attachment 365265
[details]
Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Rob Buis
Comment 49
2019-03-20 04:05:02 PDT
Created
attachment 365334
[details]
Patch
EWS Watchlist
Comment 50
2019-03-20 05:47:09 PDT
Comment on
attachment 365334
[details]
Patch
Attachment 365334
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11579141
New failing tests: http/tests/cache/link-prefetch-main-resource.php http/tests/cache/memory-cache-pruning.html
EWS Watchlist
Comment 51
2019-03-20 05:47:21 PDT
Created
attachment 365340
[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
EWS Watchlist
Comment 52
2019-03-20 06:19:51 PDT
Comment on
attachment 365334
[details]
Patch
Attachment 365334
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11579379
New failing tests: http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
EWS Watchlist
Comment 53
2019-03-20 06:19:54 PDT
Created
attachment 365342
[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
Rob Buis
Comment 54
2019-03-20 14:30:44 PDT
Created
attachment 365401
[details]
Patch
EWS Watchlist
Comment 55
2019-03-20 18:38:07 PDT
Comment on
attachment 365401
[details]
Patch
Attachment 365401
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11588645
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 56
2019-03-20 18:38:20 PDT
Created
attachment 365460
[details]
Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
youenn fablet
Comment 57
2019-03-20 18:41:53 PDT
Comment on
attachment 365401
[details]
Patch This goes in the right direction. Some suggestions below. We probably want to solve properly the case of being prefetched resources that match navigation loads. View in context:
https://bugs.webkit.org/attachment.cgi?id=365401&action=review
> Source/WebCore/loader/LinkLoader.cpp:279 > ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions();
I think we want the load to not include any credential. Also, we might want to NOT follow redirections and skip service workers. Tests might be useful there.
> Source/WebCore/loader/LinkLoader.cpp:281 > + options.certificateInfoPolicy = CertificateInfoPolicy::IncludeCertificateInfo;
Is it needed because this might fail some cache checks in NetworkProcess?
> Source/WebCore/loader/cache/CachedResourceLoader.cpp:808 > + if (frame() && frame()->page() && m_documentLoader && type != CachedResource::Type::LinkPrefetch) {
Why should we skip content extension rules for prefetch loads?
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:215 > + if (auto entry = PrefetchCache::singleton().take(request.url(), sessionID())) {
The prefetchCache should probably be owned by NetworkProcess instead of being a singleton. NetworkResourceLoader can access it through its connection
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:217 > + m_cache->store(request, entry->response(), WTFMove(buffer), nullptr);
Maybe we should have entry->releaseBuffer() instead of entry->buffer() and then WTFMove.
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:767 > + auto request = originalRequest();
Why do we need this copy?
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:768 > + if (request.httpHeaderField(HTTPHeaderName::Purpose) == "prefetch" && (!m_parameters.sourceOrigin || !m_parameters.sourceOrigin->canRequest(request.url()))) {
I think there should always be a m_parameters.sourceOrigin for prefetch resources.
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:770 > + return;
This sounds good as a first approach. If the navigation load kicks in before the prefetch is finished, the prefetch will not be used. Can we improve this?
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:36 > + : m_response(response), m_buffer(WTFMove(buffer))
We could take a ResourceResponse&&
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:47 > + : m_expirationTimer(*this, &PrefetchCache::expiration)
We could have a better name for expiration.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:96 > + ASSERT(resources->contains(requestUrl));
Is this guaranteed if we take the requestUrl in between?
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:74 > + typedef Vector<std::tuple<PAL::SessionID, URL, WallTime>> SessionPrefetchExpirationList;
Could be a Deque?
Rob Buis
Comment 58
2019-03-21 08:40:47 PDT
Created
attachment 365546
[details]
Patch
Rob Buis
Comment 59
2019-03-21 10:06:24 PDT
Created
attachment 365562
[details]
Patch
Rob Buis
Comment 60
2019-03-21 12:13:18 PDT
Comment on
attachment 365401
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=365401&action=review
>> Source/WebCore/loader/LinkLoader.cpp:279 >> ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions(); > > I think we want the load to not include any credential. > Also, we might want to NOT follow redirections and skip service workers. > Tests might be useful there.
Ah, I remember now the discussion on the github issue. Should be fixed.
>> Source/WebCore/loader/LinkLoader.cpp:281 >> + options.certificateInfoPolicy = CertificateInfoPolicy::IncludeCertificateInfo; > > Is it needed because this might fail some cache checks in NetworkProcess?
Correct, this hits "retrieveCacheEntry: Resource does not have required certificate" in NetworkResourceLoader.cpp.
>> Source/WebCore/loader/cache/CachedResourceLoader.cpp:808 >> + if (frame() && frame()->page() && m_documentLoader && type != CachedResource::Type::LinkPrefetch) { > > Why should we skip content extension rules for prefetch loads?
We hit an ASSERT in toResourceType in Debug, even without my patch, on prefetches. My thinking was that content extension would be done on the actual navigate anyway.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:215 >> + if (auto entry = PrefetchCache::singleton().take(request.url(), sessionID())) { > > The prefetchCache should probably be owned by NetworkProcess instead of being a singleton. > NetworkResourceLoader can access it through its connection
Done.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:217 >> + m_cache->store(request, entry->response(), WTFMove(buffer), nullptr); > > Maybe we should have entry->releaseBuffer() instead of entry->buffer() and then WTFMove.
Done.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:767 >> + auto request = originalRequest(); > > Why do we need this copy?
It is was not my intention to make a copy, sorry! Fixed.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:768 >> + if (request.httpHeaderField(HTTPHeaderName::Purpose) == "prefetch" && (!m_parameters.sourceOrigin || !m_parameters.sourceOrigin->canRequest(request.url()))) { > > I think there should always be a m_parameters.sourceOrigin for prefetch resources.
Ok I removed the check.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:770 >> + return; > > This sounds good as a first approach. > If the navigation load kicks in before the prefetch is finished, the prefetch will not be used. > Can we improve this?
It may be possible if we track prefetches as soon as possible, i.e when the request is sent. Then when the navigation load is finishing up, there could be a check to see if any prefetch is pending and it could be cancelled. I will try tomorrow, but not too sure about actually testing this.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:36 >> + : m_response(response), m_buffer(WTFMove(buffer)) > > We could take a ResourceResponse&&
Done.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:47 >> + : m_expirationTimer(*this, &PrefetchCache::expiration) > > We could have a better name for expiration.
I am still thinking :) I'll probably have a better name tomorrow.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:96 >> + ASSERT(resources->contains(requestUrl)); > > Is this guaranteed if we take the requestUrl in between?
I think it is okay because of how PrefetchCache::take works. But I am fine with making these if checks, to be on the safe side.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:74 >> + typedef Vector<std::tuple<PAL::SessionID, URL, WallTime>> SessionPrefetchExpirationList; > > Could be a Deque?
Done.
EWS Watchlist
Comment 61
2019-03-21 12:36:04 PDT
Comment on
attachment 365562
[details]
Patch
Attachment 365562
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11602069
New failing tests: http/tests/cache/link-prefetch-main-resource.php
EWS Watchlist
Comment 62
2019-03-21 12:36:16 PDT
Created
attachment 365591
[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
EWS Watchlist
Comment 63
2019-03-21 17:13:11 PDT
Comment on
attachment 365562
[details]
Patch
Attachment 365562
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11606201
New failing tests: http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
EWS Watchlist
Comment 64
2019-03-21 17:13:13 PDT
Created
attachment 365649
[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Yoav Weiss
Comment 65
2019-03-21 21:47:33 PDT
Comment on
attachment 365562
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=365562&action=review
> Source/WebCore/loader/LinkLoader.cpp:284 > + options.mode = FetchOptions::Mode::Cors;
The current spec PR[1] doesn't automagically set CORS mode, credentials mode and referrer policy, but verifies that they are properly set (e.g. via crossorigin and referrerpolicy attributes) [1]
https://whatpr.org/html/4115/e32a6f8...e4193e2/semantics.html#process-the-linked-resource
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:55 > +static const Seconds expirationTimeout { 5_s };
5 seconds expiration seems rather low
Yoav Weiss
Comment 66
2019-03-21 21:51:04 PDT
(In reply to Yoav Weiss from
comment #65
)
> Comment on
attachment 365562
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=365562&action=review
> > > Source/WebCore/loader/LinkLoader.cpp:284 > > + options.mode = FetchOptions::Mode::Cors; > > The current spec PR[1] doesn't automagically set CORS mode, credentials mode > and referrer policy, but verifies that they are properly set (e.g. via > crossorigin and referrerpolicy attributes)
Regarding Omit mode, that's not what the spec PR currently says, so I wonder if this is required (it would render some same-origin prefetch use-cases rather useless, e.g. prefetching the next step in a shopping cart funnel). If it is required, we would need to add a `crossorigin` "omit" value, so that it can be set with an attribute.
> > [1] >
https://whatpr.org/html/4115/e32a6f8...e4193e2/semantics.html#process-the
- > linked-resource > > > Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:55 > > +static const Seconds expirationTimeout { 5_s }; > > 5 seconds expiration seems rather low
Rob Buis
Comment 67
2019-03-22 02:07:38 PDT
Created
attachment 365697
[details]
Patch
Rob Buis
Comment 68
2019-03-22 02:22:52 PDT
Comment on
attachment 365562
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=365562&action=review
>>> Source/WebCore/loader/LinkLoader.cpp:284 >>> + options.mode = FetchOptions::Mode::Cors; >> >> The current spec PR[1] doesn't automagically set CORS mode, credentials mode and referrer policy, but verifies that they are properly set (e.g. via crossorigin and referrerpolicy attributes) >> >> [1]
https://whatpr.org/html/4115/e32a6f8...e4193e2/semantics.html#process-the-linked-resource
> > Regarding Omit mode, that's not what the spec PR currently says, so I wonder if this is required (it would render some same-origin prefetch use-cases rather useless, e.g. prefetching the next step in a shopping cart funnel). > > If it is required, we would need to add a `crossorigin` "omit" value, so that it can be set with an attribute.
So in general here we have to set up decent default values that work for prefetches. Not sure if adding a `crossorigin` should be part of this ppatch.
>>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:55 >>> +static const Seconds expirationTimeout { 5_s }; >> >> 5 seconds expiration seems rather low > >
It is certainly up for discussion :) I had in the back of my mind, I would like to write a test for the timeout, but with a large timeout, it becomes a not so attractive test. But apart from that practical problem, of course we can increase it.
EWS Watchlist
Comment 69
2019-03-22 03:27:15 PDT
Comment on
attachment 365697
[details]
Patch
Attachment 365697
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
https://webkit-queues.webkit.org/results/11611210
New failing tests: imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html
EWS Watchlist
Comment 70
2019-03-22 03:27:18 PDT
Created
attachment 365700
[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
EWS Watchlist
Comment 71
2019-03-22 04:05:29 PDT
Comment on
attachment 365697
[details]
Patch
Attachment 365697
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11611525
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 72
2019-03-22 04:05:41 PDT
Created
attachment 365705
[details]
Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Yoav Weiss
Comment 73
2019-03-22 05:52:43 PDT
(In reply to Rob Buis from
comment #68
)
> Comment on
attachment 365562
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=365562&action=review
> > >>> Source/WebCore/loader/LinkLoader.cpp:284 > >>> + options.mode = FetchOptions::Mode::Cors; > >> > >> The current spec PR[1] doesn't automagically set CORS mode, credentials mode and referrer policy, but verifies that they are properly set (e.g. via crossorigin and referrerpolicy attributes) > >> > >> [1]
https://whatpr.org/html/4115/e32a6f8...e4193e2/semantics.html#process-the-linked-resource
> > > > Regarding Omit mode, that's not what the spec PR currently says, so I wonder if this is required (it would render some same-origin prefetch use-cases rather useless, e.g. prefetching the next step in a shopping cart funnel). > > > > If it is required, we would need to add a `crossorigin` "omit" value, so that it can be set with an attribute. > > So in general here we have to set up decent default values that work for > prefetches. Not sure if adding a `crossorigin` should be part of this ppatch.
Note that for preload, we have avoided setting "magical" values (e.g. for "font" destinations), and left that as extra work (but more predictable) to the developer. I'm open to discuss that decision though. Mind commenting on the PR[1], saying that you'd prefer setting the values, rather than checking for them. Youenn - thoughts on the above would be appreciated as well [1]
https://github.com/whatwg/html/pull/4115
> > >>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:55 > >>> +static const Seconds expirationTimeout { 5_s }; > >> > >> 5 seconds expiration seems rather low > > > > > > It is certainly up for discussion :) I had in the back of my mind, I would > like to write a test for the timeout, but with a large timeout, it becomes a > not so attractive test. But apart from that practical problem, of course we > can increase it.
Ryosuke Niwa
Comment 74
2019-03-22 19:57:03 PDT
Comment on
attachment 365697
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=365697&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:35 > +PrefetchEntry::PrefetchEntry(WebCore::ResourceResponse&& response, RefPtr<WebCore::SharedBuffer>&& buffer)
We need to clear this cache upon receiving memory pressure.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:87 > + std::tie(sessionID, requestUrl, timestamp) = m_sessionExpirationList.first();
Use .random()
Ryosuke Niwa
Comment 75
2019-03-22 19:58:53 PDT
It looks like it's unsound to prefetch cross-site because the content can be read with Spectre class of attacks. We'd have to solve that problem before we can ever enable this feature.
youenn fablet
Comment 76
2019-03-24 07:01:47 PDT
> We need to clear this cache upon receiving memory pressure.
Good point. (In reply to Ryosuke Niwa from
comment #75
)
> It looks like it's unsound to prefetch cross-site because the content can be > read with Spectre class of attacks. We'd have to solve that problem before > we can ever enable this feature.
The prefetch cache is in NetworkProcess. The plan is to only use it for now for navigations so only the right process should get the response. Should we extend prefetch to other loads (I dislike this though), we should apply the regular sanitization including CORS and CORP as done for other resources coming from network or from network cache.
youenn fablet
Comment 77
2019-03-25 03:48:51 PDT
We might also want to limit prefetches for now, maybe with something like: - third-party iframes cannot trigger prefetches - Number of prefetches of a given page is limited (to 1 maybe?) I would add a FIXME in LinkLoader::prefetchIfNeeded about this. SameOrigin for credentials is probably ok. CORS mode does not make a lot of sense in that context. We actually do not want to trigger CORS or CORP checks on the response since the target is navigation loads. FetchOptions::Mode::Navigate makes some sense but has some implications with regards to our implementation. Probably that is the one making most sense currently since we will treat it as if it is a top level navigation.
youenn fablet
Comment 78
2019-03-25 09:06:05 PDT
I wonder how/whether we want want load/error events. Should load event be fired when load starts or when load finishes? Can the timing of the load event leak some information if done at load finish? If load fails, should error event be sent? If we want to support load/error events when load finishes, we might need to provide an opaque response to the web process. This might be best handled by a new mode, so that NetworkLoadChecker does the necessary sanitization. And we do not want the body to be transmitted in any case. One safe way might be to fire the load event whenever the load starts and the error event if the load is not even started. Ideally, we would then directly return didReceiveResponse/didFinishLoading. But we will then need to handle the case of loads that last longer than what the WebProcess thinks they do.
Rob Buis
Comment 79
2019-03-25 09:44:57 PDT
Created
attachment 365869
[details]
Patch
Rob Buis
Comment 80
2019-03-25 09:46:40 PDT
(In reply to youenn fablet from
comment #76
)
> > We need to clear this cache upon receiving memory pressure. > > Good point.
Agreed, I added code for this in the latest patch. Note that I am not sure how to test it though.
Rob Buis
Comment 81
2019-03-25 09:48:21 PDT
(In reply to youenn fablet from
comment #77
)
> We might also want to limit prefetches for now, maybe with something like: > - third-party iframes cannot trigger prefetches > - Number of prefetches of a given page is limited (to 1 maybe?) > I would add a FIXME in LinkLoader::prefetchIfNeeded about this.
I added the FIXME.
> SameOrigin for credentials is probably ok.
Using SameOrigin now.
> CORS mode does not make a lot of sense in that context. > We actually do not want to trigger CORS or CORP checks on the response since > the target is navigation loads. > FetchOptions::Mode::Navigate makes some sense but has some implications with > regards to our implementation. Probably that is the one making most sense > currently since we will treat it as if it is a top level navigation.
Using FetchOptions::Mode::Navigate now.
youenn fablet
Comment 82
2019-03-25 09:49:45 PDT
(In reply to Rob Buis from
comment #80
)
> (In reply to youenn fablet from
comment #76
) > > > We need to clear this cache upon receiving memory pressure. > > > > Good point. > > Agreed, I added code for this in the latest patch. Note that I am not sure > how to test it though.
We might need a testRunner API to trigger memory pressure in Network Process. This might be done in a follow-up patch though, w/o the memory pressure cleanup.
EWS Watchlist
Comment 83
2019-03-25 12:15:09 PDT
Comment on
attachment 365869
[details]
Patch
Attachment 365869
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11661967
New failing tests: http/tests/cache/link-prefetch-main-resource.html
EWS Watchlist
Comment 84
2019-03-25 12:15:22 PDT
Created
attachment 365880
[details]
Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Rob Buis
Comment 85
2019-03-25 12:33:06 PDT
Created
attachment 365881
[details]
Patch
Rob Buis
Comment 86
2019-03-26 12:38:20 PDT
Created
attachment 365987
[details]
Patch
Rob Buis
Comment 87
2019-03-26 14:42:42 PDT
Created
attachment 366000
[details]
Patch
youenn fablet
Comment 88
2019-03-26 15:13:18 PDT
Comment on
attachment 366000
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=366000&action=review
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:213 > + if (m_parameters.options.mode == FetchOptions::Mode::Navigate) {
We probably want to restrict to top level navigation for now.
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:768 > + }
We should probably add a check on didReceiveResponse/didReceiveBuffer to not send data if it is a prefetch request.
> LayoutTests/http/tests/cache/link-prefetch-main-resource.html:1 > +<!-- webkit-test-runner [ experimental:LinkPrefetch=false ] -->
Should be true?
> LayoutTests/http/tests/cache/link-prefetch-main-resource.html:10 > + internals.settings.setStorageBlockingPolicy('BlockThirdParty')
Why is it needed?
> LayoutTests/http/tests/cache/link-prefetch-main-resource.html:40 > +<link rel="prefetch" href="
http://localhost:8000/cache/resources/prefetched-main-resource.php
" onload="setTimeout(loadAfterPrefetch, 0);">
We might want to contribute these tests to WPT maybe.
> LayoutTests/http/tests/cache/resources/prefetched-main-resource.php:4 > + header("Access-Control-Allow-Origin:
http://127.0.0.1:8000
");
Maybe we should have echo "parent.window.postMessage('PASS', '*');";
> LayoutTests/http/tests/cache/resources/prefetched-main-resource.php:12 > +echo "parent.window.postMessage('nonprefetch', '*');";
Maybe we should have echo "parent.window.postMessage('FAIL', '*');";
EWS Watchlist
Comment 89
2019-03-27 00:01:55 PDT
Comment on
attachment 366000
[details]
Patch
Attachment 366000
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/11680402
New failing tests: fast/visual-viewport/ios/min-scale-greater-than-one.html
EWS Watchlist
Comment 90
2019-03-27 00:01:57 PDT
Created
attachment 366057
[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
Yoav Weiss
Comment 91
2019-03-27 01:40:20 PDT
(In reply to youenn fablet from
comment #78
)
> I wonder how/whether we want want load/error events. > Should load event be fired when load starts or when load finishes? Can the > timing of the load event leak some information if done at load finish? > If load fails, should error event be sent? > > If we want to support load/error events when load finishes, we might need to > provide an opaque response to the web process. This might be best handled by > a new mode, so that NetworkLoadChecker does the necessary sanitization. And > we do not want the body to be transmitted in any case. > > One safe way might be to fire the load event whenever the load starts and > the error event if the load is not even started.
Firing at load start would diverge from typical `load` event semantics. The opaque response approach seems best to me.
> > Ideally, we would then directly return didReceiveResponse/didFinishLoading. > But we will then need to handle the case of loads that last longer than what > the WebProcess thinks they do.
Rob Buis
Comment 92
2019-03-27 07:39:06 PDT
Created
attachment 366069
[details]
Patch
Rob Buis
Comment 93
2019-03-27 09:26:40 PDT
Comment on
attachment 366000
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=366000&action=review
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:213 >> + if (m_parameters.options.mode == FetchOptions::Mode::Navigate) { > > We probably want to restrict to top level navigation for now.
Done.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:768 >> + } > > We should probably add a check on didReceiveResponse/didReceiveBuffer to not send data if it is a prefetch request.
Done.
>> LayoutTests/http/tests/cache/link-prefetch-main-resource.html:1 >> +<!-- webkit-test-runner [ experimental:LinkPrefetch=false ] --> > > Should be true?
I think it has no effect, probably since link prefetch is an experimental flag and will get enabled on test runs by default, so I removed the line.
>> LayoutTests/http/tests/cache/link-prefetch-main-resource.html:10 >> + internals.settings.setStorageBlockingPolicy('BlockThirdParty') > > Why is it needed?
I think this was needed at some point but it does not have an effect anymore, I removed it.
>> LayoutTests/http/tests/cache/link-prefetch-main-resource.html:40 >> +<link rel="prefetch" href="
http://localhost:8000/cache/resources/prefetched-main-resource.php
" onload="setTimeout(loadAfterPrefetch, 0);"> > > We might want to contribute these tests to WPT maybe.
I hope they are good enough :)
>> LayoutTests/http/tests/cache/resources/prefetched-main-resource.php:4 >> + header("Access-Control-Allow-Origin:
http://127.0.0.1:8000
"); > > Maybe we should have echo "parent.window.postMessage('PASS', '*');";
Done.
>> LayoutTests/http/tests/cache/resources/prefetched-main-resource.php:12 >> +echo "parent.window.postMessage('nonprefetch', '*');"; > > Maybe we should have echo "parent.window.postMessage('FAIL', '*');";
Done.
youenn fablet
Comment 94
2019-03-28 22:34:33 PDT
Comment on
attachment 366069
[details]
Patch This seems almost ready. Some comments and suggestions. View in context:
https://bugs.webkit.org/attachment.cgi?id=366069&action=review
> Source/WebCore/loader/LinkLoader.cpp:279 > + // FIXME further prefetch limitations:
s/FIXME further/FIXME: Add further prefetch restrictions/
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:470 > + send(Messages::WebResourceLoader::DidReceiveResponse { response, willWaitForContinueDidReceiveResponse });
So, the idea would be to keep sending the prefetch to the memory cache if same origin. That makes some sense for optimizations. The fetch PR for defining is not really aligned but we could reconcile both progressively. My question is what happens for cross origin prefetch in the web process? I guess they do not go in the memory cache. Do we have cases of loading where we go directly from send request to didFinishLoading without going through didReceiveResponse? It seems we could do: if (isCrossOriginPrefetch(originalRequest()) { send(Messages::WebResourceLoader::DidReceiveResponse { ResourceResponse { }, false }); return completionHandler(PolicyAction::Use); }
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:67 > +static const Seconds expirationTimeout { 5_s };
We might want to update this in the future, 5 seconds might not be long enough. I wonder whether it should be something like 5_s after the page doing the prefetch navigated away. This might add some complexity to the implementation though. Not for this patch.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:89 > + return *map;
We use m_sessionPrefetches.ensure nowadays.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:59 > + void clear();
We might need to clear part of the prefetch cache in NetworkProcess::destroySession.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:61 > + std::unique_ptr<PrefetchEntry> take(URL, PAL::SessionID);
const URL&. I believe we usually pass SessionID parameter first.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:62 > + void store(URL, WebCore::ResourceResponse&&, RefPtr<WebCore::SharedBuffer>&&, PAL::SessionID);
URL&&
> LayoutTests/platform/mac-wk1/TestExpectations:699 > +
webkit.org/b/195623
http/tests/cache/link-prefetch-main-resource.html [ Failure ]
Let's skip it as well since we miss the implementation.
> LayoutTests/platform/win/TestExpectations:4308 > +
webkit.org/b/195623
http/tests/cache/link-prefetch-main-resource.html [ Timeout ]
Let's skip it then
youenn fablet
Comment 95
2019-03-28 22:36:39 PDT
Btw, the test is doing an iframe load. Should we do a top level navigation prefetch load instead?
youenn fablet
Comment 96
2019-03-28 22:52:19 PDT
Rethinking about it, we do not want cross origin prefetch to end in the memory cache with an empty response. Maybe that was your original idea? I would anyway make it clear that compeltionHandler is called with use in any prefetch
Rob Buis
Comment 97
2019-03-29 04:56:34 PDT
Created
attachment 366266
[details]
Patch
EWS Watchlist
Comment 98
2019-03-29 04:58:29 PDT
Attachment 366266
[details]
did not pass style-queue: ERROR: LayoutTests/platform/win/TestExpectations:4306: Path does not exist. [test/expectations] [5] ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/mac-wk1/TestExpectations:699: Path does not exist. [test/expectations] [5] ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/win/TestExpectations:4306: Path does not exist. [test/expectations] [5] Total errors found: 3 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Rob Buis
Comment 99
2019-03-29 08:37:23 PDT
Created
attachment 366273
[details]
Patch
EWS Watchlist
Comment 100
2019-03-29 09:45:45 PDT
Comment on
attachment 366273
[details]
Patch
Attachment 366273
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11703243
New failing tests: http/tests/cache/link-prefetch-main-resource-iframe.html
EWS Watchlist
Comment 101
2019-03-29 09:45:47 PDT
Created
attachment 366278
[details]
Archive of layout-test-results from ews103 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
Rob Buis
Comment 102
2019-03-29 09:49:13 PDT
Comment on
attachment 366069
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=366069&action=review
>> Source/WebCore/loader/LinkLoader.cpp:279 >> + // FIXME further prefetch limitations: > > s/FIXME further/FIXME: Add further prefetch restrictions/
Done.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:470 >> + send(Messages::WebResourceLoader::DidReceiveResponse { response, willWaitForContinueDidReceiveResponse }); > > So, the idea would be to keep sending the prefetch to the memory cache if same origin. > That makes some sense for optimizations. > The fetch PR for defining is not really aligned but we could reconcile both progressively. > > My question is what happens for cross origin prefetch in the web process? > I guess they do not go in the memory cache. > > Do we have cases of loading where we go directly from send request to didFinishLoading without going through didReceiveResponse? > It seems we could do: > if (isCrossOriginPrefetch(originalRequest()) { > send(Messages::WebResourceLoader::DidReceiveResponse { ResourceResponse { }, false }); > return completionHandler(PolicyAction::Use); > }
I don't think we have cases of didFinishLoading without going through didReceiveResponse. Sending an empty ResourceResponse actually hits an ASSERT on WebCore side. I don't think it is too important if the prefetch ends up in the memory cache as it will be a cache miss anyway (partitioning). But I'll think on this further.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:67 >> +static const Seconds expirationTimeout { 5_s }; > > We might want to update this in the future, 5 seconds might not be long enough. > I wonder whether it should be something like 5_s after the page doing the prefetch navigated away. > This might add some complexity to the implementation though. > Not for this patch.
Ok.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:89 >> + return *map; > > We use m_sessionPrefetches.ensure nowadays.
Oops, forgot about this, will try to fix it soon.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:59 >> + void clear(); > > We might need to clear part of the prefetch cache in NetworkProcess::destroySession.
When would that be useful?
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:61 >> + std::unique_ptr<PrefetchEntry> take(URL, PAL::SessionID); > > const URL&. > I believe we usually pass SessionID parameter first.
Done.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:62 >> + void store(URL, WebCore::ResourceResponse&&, RefPtr<WebCore::SharedBuffer>&&, PAL::SessionID); > > URL&&
I think that makes things more complex so I left it off for now.
>> LayoutTests/platform/mac-wk1/TestExpectations:699 >> +
webkit.org/b/195623
http/tests/cache/link-prefetch-main-resource.html [ Failure ] > > Let's skip it as well since we miss the implementation.
Done.
>> LayoutTests/platform/win/TestExpectations:4308 >> +
webkit.org/b/195623
http/tests/cache/link-prefetch-main-resource.html [ Timeout ] > > Let's skip it then
Done.
Rob Buis
Comment 103
2019-03-29 09:50:37 PDT
(In reply to youenn fablet from
comment #95
)
> Btw, the test is doing an iframe load. > Should we do a top level navigation prefetch load instead?
Yes that sounds useful. I added such a test and renamed the iframe test to make it more clear it is using an iframe.
Rob Buis
Comment 104
2019-03-29 09:54:54 PDT
(In reply to youenn fablet from
comment #96
)
> Rethinking about it, we do not want cross origin prefetch to end in the > memory cache with an empty response. Maybe that was your original idea? > I would anyway make it clear that compeltionHandler is called with use in > any prefetch
My first patches were concentrated on the memory cache which turned out to be the wrong approach. Since then I have not been too concerned with it because of the cache miss. I assume at this point making cached result empty is kind of efficient, compared to storing the full result. Do we even really need an entry for prefetches in the memory cache? Again I need to think about it, but let me know if you get convinced of the best way to go here.
EWS Watchlist
Comment 105
2019-03-29 10:25:39 PDT
Comment on
attachment 366273
[details]
Patch
Attachment 366273
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/11703360
New failing tests: http/tests/cache/link-prefetch-main-resource-iframe.html
EWS Watchlist
Comment 106
2019-03-29 10:25:51 PDT
Created
attachment 366280
[details]
Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
EWS Watchlist
Comment 107
2019-03-29 10:35:20 PDT
Comment on
attachment 366273
[details]
Patch
Attachment 366273
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11703301
New failing tests: http/tests/cache/link-prefetch-main-resource-iframe.html
EWS Watchlist
Comment 108
2019-03-29 10:35:24 PDT
Created
attachment 366281
[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
Rob Buis
Comment 109
2019-04-02 01:44:48 PDT
Created
attachment 366476
[details]
Patch
EWS Watchlist
Comment 110
2019-04-02 02:53:10 PDT
Comment on
attachment 366476
[details]
Patch
Attachment 366476
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11737128
New failing tests: http/tests/cache/link-prefetch-main-resource-iframe.html
EWS Watchlist
Comment 111
2019-04-02 02:53:12 PDT
Created
attachment 366479
[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
EWS Watchlist
Comment 112
2019-04-02 03:34:59 PDT
Comment on
attachment 366476
[details]
Patch
Attachment 366476
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11737203
New failing tests: http/tests/cache/link-prefetch-main-resource-iframe.html
EWS Watchlist
Comment 113
2019-04-02 03:35:02 PDT
Created
attachment 366481
[details]
Archive of layout-test-results from ews117 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-highsierra Platform: Mac OS X 10.13.6
Rob Buis
Comment 114
2019-04-02 05:11:20 PDT
Created
attachment 366488
[details]
Patch
youenn fablet
Comment 115
2019-04-02 15:50:50 PDT
> My first patches were concentrated on the memory cache which turned out to > be the wrong approach. Since then I have not been too concerned with it > because of the cache miss. I assume at this point making cached result empty > is kind of efficient, compared to storing the full result. Do we even really > need an entry for prefetches in the memory cache? Again I need to think > about it, but let me know if you get convinced of the best way to go here.
We certainly do not want cross-origin prefetches to go in the memory cache. We could be ok with same-origin prefetches to go in the memory cache but I feel like it might not be worth the added complexity.
youenn fablet
Comment 116
2019-04-02 16:03:56 PDT
Comment on
attachment 366488
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=366488&action=review
> Source/WebCore/ChangeLog:14 > + http/tests/cache/link-prefetch-main-resource.html
Double line.
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:213 > + if (isMainResource() && m_parameters.options.mode == FetchOptions::Mode::Navigate) {
Let's restrict it to isMainFrameLoad(). I would think iframes do not need to opt out of partitioning and preload could be used instead for these.
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:467 > + send(Messages::WebResourceLoader::DidReceiveResponse { response, false });
I think we should do ResourceResponse { } instead of passing response.
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:770 > + const auto& request = originalRequest();
request is only used once, maybe we can remove it.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:64 > + return resources->take(url);
Do we need the call to resources->contains(). If not there, I would believe take will return nullptr.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:92 > +void PrefetchCache::expiration()
How about clearExpiredEntries?
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:36 > +#include <wtf/text/WTFString.h>
CompletionHandler.h/Seconds.h do not seem needed.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:50 > + mutable RefPtr<WebCore::SharedBuffer> m_buffer;
We can probably simplify this by removing mutable and making releaseBuffer() not const.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:54 > + WTF_MAKE_NONCOPYABLE(PrefetchCache); WTF_MAKE_FAST_ALLOCATED;
Add a \n
Rob Buis
Comment 117
2019-04-03 01:39:21 PDT
Created
attachment 366585
[details]
Patch
Rob Buis
Comment 118
2019-04-03 01:59:15 PDT
Created
attachment 366586
[details]
Patch
Rob Buis
Comment 119
2019-04-03 07:38:02 PDT
Created
attachment 366602
[details]
Patch
Rob Buis
Comment 120
2019-04-03 07:39:34 PDT
Comment on
attachment 366488
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=366488&action=review
>> Source/WebCore/ChangeLog:14 >> + http/tests/cache/link-prefetch-main-resource.html > > Double line.
Fixed.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:213 >> + if (isMainResource() && m_parameters.options.mode == FetchOptions::Mode::Navigate) { > > Let's restrict it to isMainFrameLoad(). > I would think iframes do not need to opt out of partitioning and preload could be used instead for these.
Done.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:467 >> + send(Messages::WebResourceLoader::DidReceiveResponse { response, false }); > > I think we should do ResourceResponse { } instead of passing response.
Done.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:770 >> + const auto& request = originalRequest(); > > request is only used once, maybe we can remove it.
Done.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:64 >> + return resources->take(url); > > Do we need the call to resources->contains(). > If not there, I would believe take will return nullptr.
Right, not needed.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:92 >> +void PrefetchCache::expiration() > > How about clearExpiredEntries?
Sorry, I forgot about this issue! I like the name, done.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:36 >> +#include <wtf/text/WTFString.h> > > CompletionHandler.h/Seconds.h do not seem needed.
Done.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:50 >> + mutable RefPtr<WebCore::SharedBuffer> m_buffer; > > We can probably simplify this by removing mutable and making releaseBuffer() not const.
Done.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:54 >> + WTF_MAKE_NONCOPYABLE(PrefetchCache); WTF_MAKE_FAST_ALLOCATED; > > Add a \n
Done.
Rob Buis
Comment 121
2019-04-03 07:42:50 PDT
(In reply to youenn fablet from
comment #115
)
> > My first patches were concentrated on the memory cache which turned out to > > be the wrong approach. Since then I have not been too concerned with it > > because of the cache miss. I assume at this point making cached result empty > > is kind of efficient, compared to storing the full result. Do we even really > > need an entry for prefetches in the memory cache? Again I need to think > > about it, but let me know if you get convinced of the best way to go here. > > We certainly do not want cross-origin prefetches to go in the memory cache. > We could be ok with same-origin prefetches to go in the memory cache but I > feel like it might not be worth the added complexity.
Okay, thanks for clarifying. In the latest patch I now use CachingPolicy::DisallowCaching to not use the memory cache for prefetches at all.
youenn fablet
Comment 122
2019-04-12 08:34:00 PDT
Comment on
attachment 366602
[details]
Patch r=me LGTM. Some other things we might need: - Web Inspector integration? - Handling of still loading prefetched responses View in context:
https://bugs.webkit.org/attachment.cgi?id=366602&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:34 > +#include <wtf/text/WTFString.h>
probably not needed.
Darin Adler
Comment 123
2019-04-12 08:59:35 PDT
Comment on
attachment 366602
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=366602&action=review
> Source/WebCore/loader/ResourceLoadInfo.h:47 > + Prefetch = 0x0400, > }; > const uint16_t ResourceTypeMask = 0x03FF;
Seems dangerous that there’s a new type that’s not in ResourceTypeMask. Shouldn’t this have been changed to 0x7FF?
Darin Adler
Comment 124
2019-04-12 09:03:12 PDT
Comment on
attachment 366602
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=366602&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:46 > + clear();
This seems unnecessary. I don’t see any work done in clear() that has any value if the objects are being destroyed immediately afterward. Seems like this will just make the destructor slower with no benefit.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:86 > + auto& map = m_sessionPrefetches.add(sessionID, nullptr).iterator->value; > + if (!map) > + map = std::make_unique<PrefetchEntriesMap>(); > + return *map;
Could use HashMap::ensure instead of add to avoid the if statement.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:49 > +class PrefetchEntry { > + WTF_MAKE_FAST_ALLOCATED; > +public: > + PrefetchEntry(WebCore::ResourceResponse&&, RefPtr<WebCore::SharedBuffer>&&); > + > + const WebCore::ResourceResponse& response() const { return m_response; } > + RefPtr<WebCore::SharedBuffer> releaseBuffer() { return m_buffer.releaseNonNull(); } > + > +private: > + WebCore::ResourceResponse m_response; > + RefPtr<WebCore::SharedBuffer> m_buffer; > +};
Why a class instead of a struct for this? I think it would be better with a struct. I also think this should be a member instead of a separate struct. PrefetchCache::Entry.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:66 > + typedef HashMap<URL, std::unique_ptr<PrefetchEntry>> PrefetchEntriesMap;
New code should us "using" instead of typedef.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:70 > + typedef HashMap<PAL::SessionID, std::unique_ptr<PrefetchEntriesMap>> SessionPrefetchResourceMap;
Ditto.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:73 > + typedef Deque<std::tuple<PAL::SessionID, URL, WallTime>> SessionPrefetchExpirationList;
Ditto.
Rob Buis
Comment 125
2019-04-13 05:07:39 PDT
Created
attachment 367385
[details]
Patch
Darin Adler
Comment 126
2019-04-13 07:44:08 PDT
Comment on
attachment 367385
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367385&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:55 > + struct Entry { > + Entry(WebCore::ResourceResponse&&, RefPtr<WebCore::SharedBuffer>&&); > + > + const WebCore::ResourceResponse& response() const { return m_response; } > + RefPtr<WebCore::SharedBuffer> releaseBuffer() { return m_buffer.releaseNonNull(); } > + > + WebCore::ResourceResponse m_response; > + RefPtr<WebCore::SharedBuffer> m_buffer; > + };
More about what I meant when I suggested this be a struct: This struct can simply have "response" and "buffer" public data members. No need for the "m_" prefix for public struct data members. Also likely don’t need to define a constructor since aggregate construction should work. Could keep the releaseBuffer function if you like, or callers can just write buffer.releaseNonNull() directly. So ideally the struct just has two lines in its body, keeping it simpler.
Darin Adler
Comment 127
2019-04-13 07:44:38 PDT
Thanks for taking all my suggestions, by the way!
Rob Buis
Comment 128
2019-04-13 08:17:29 PDT
Created
attachment 367388
[details]
Patch
Rob Buis
Comment 129
2019-04-14 12:55:18 PDT
Created
attachment 367404
[details]
Patch
WebKit Commit Bot
Comment 130
2019-04-14 13:52:52 PDT
Comment on
attachment 367404
[details]
Patch Clearing flags on attachment: 367404 Committed
r244248
: <
https://trac.webkit.org/changeset/244248
>
WebKit Commit Bot
Comment 131
2019-04-14 13:52:55 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 132
2019-04-14 13:54:08 PDT
<
rdar://problem/49887960
>
Ryan Haddad
Comment 133
2019-04-15 08:54:01 PDT
The two tests added with this change are consistently failing an assertion on macOS and iOS Debug WK2 bots: http/tests/cache/link-prefetch-main-resource-iframe.html http/tests/cache/link-prefetch-main-resource.html ASSERTION FAILED: !response.isNull() ./loader/SubresourceLoader.cpp(315) : virtual void WebCore::SubresourceLoader::didReceiveResponse(const WebCore::ResourceResponse &, CompletionHandler<void ()> &&) 1 0x591628019 WTFCrash 2 0x57f1f112b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x581fac83f WebCore::SubresourceLoader::didReceiveResponse(WebCore::ResourceResponse const&, WTF::CompletionHandler<void ()>&&) 4 0x57913a41d WebKit::WebResourceLoader::didReceiveResponse(WebCore::ResourceResponse const&, bool) 5 0x5796d3a28 void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool), std::__1::tuple<WebCore::ResourceResponse, bool>, 0ul, 1ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool), std::__1::tuple<WebCore::ResourceResponse, bool>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) 6 0x5796d3950 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool), std::__1::tuple<WebCore::ResourceResponse, bool>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<WebCore::ResourceResponse, bool>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool)) 7 0x5796d2eee void IPC::handleMessage<Messages::WebResourceLoader::DidReceiveResponse, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool)) 8 0x5796d27f0 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&) 9 0x57912c466 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 10 0x57806272c IPC::Connection::dispatchMessage(IPC::Decoder&) 11 0x578054851 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) 12 0x5780634f7 IPC::Connection::dispatchOneIncomingMessage() 13 0x578084668 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() 14 0x578084579 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() 15 0x59165249d WTF::Function<void ()>::operator()() const 16 0x5916b4713 WTF::RunLoop::performWork() 17 0x5916b50a4 WTF::RunLoop::performWork(void*) 18 0x7fff3656cd81 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ 19 0x7fff3662465c __CFRunLoopDoSource0 20 0x7fff3654fd30 __CFRunLoopDoSources0 21 0x7fff3654f1ad __CFRunLoopRun 22 0x7fff3654ea07 CFRunLoopRunSpecific 23 0x7fff3582cd96 RunCurrentEventLoopInMode 24 0x7fff3582cb06 ReceiveNextEventCommon 25 0x7fff3582c884 _BlockUntilNextEventMatchingListInModeWithFilter 26 0x7fff33adfa73 _DPSNextEvent 27 0x7fff34275e34 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 28 0x7fff33ad4885 -[NSApplication run] 29 0x7fff33aa3a72 NSApplicationMain 30 0x7fff5ec4df57 _xpc_objc_main 31 0x7fff5ec4cbaa xpc_main LEAK: 2 WebPageProxy
https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r244259%20(7447)/results.html
Rob Buis
Comment 134
2019-04-15 10:37:47 PDT
Hi, (In reply to Ryan Haddad from
comment #133
)
> The two tests added with this change are consistently failing an assertion > on macOS and iOS Debug WK2 bots:
So mac-debug and mac-wk2 are not covering this? If so I have no real way of detecting this (except an expensive local WK2 Debug build). I think I know what is going on and can try to fix it tomorrow.
Rob Buis
Comment 135
2019-04-16 06:49:08 PDT
Reopening to attach new patch.
Rob Buis
Comment 136
2019-04-16 06:49:10 PDT
Created
attachment 367534
[details]
Patch
Ryan Haddad
Comment 137
2019-04-17 09:32:05 PDT
Is this ready for review? I'd like to roll out the breaking change if it isn't.
Rob Buis
Comment 138
2019-04-17 10:56:28 PDT
Comment on
attachment 367534
[details]
Patch Ready for review. Another option is to convert the ASSERT to an if check with early return for null response. Both options seem like no-ops to me, so it may be best to just not call it for our case.
Alex Christensen
Comment 139
2019-04-17 11:21:43 PDT
Comment on
attachment 367534
[details]
Patch If that fixes the assertion and doesn't cause any more problems, r=me.
Ryan Haddad
Comment 140
2019-04-17 16:01:40 PDT
Comment on
attachment 367534
[details]
Patch Marking cq+ since EWS is green.
WebKit Commit Bot
Comment 141
2019-04-17 16:29:50 PDT
Comment on
attachment 367534
[details]
Patch Clearing flags on attachment: 367534 Committed
r244409
: <
https://trac.webkit.org/changeset/244409
>
WebKit Commit Bot
Comment 142
2019-04-17 16:29:54 PDT
All reviewed patches have been landed. Closing bug.
Shawn Roberts
Comment 143
2019-04-18 08:57:11 PDT
It looks like after
https://trac.webkit.org/changeset/244409
http/tests/cache/memory-cache-pruning.html is crashing on Mac WK2 Debug builds.
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fcache%2Fmemory-cache-pruning.html
Trying to reproduce locally, it may be triggering off another test. Will get repro steps soon hopefully. It appears related tho. ERROR: Unhandled web process message 'WebUserContentController:RemoveAllContentRuleLists' /Volumes/Data/slave/mojave-debug/build/Source/WebKit/WebProcess/WebProcess.cpp(762) : virtual void WebKit::WebProcess::didReceiveMessage(IPC::Connection &, IPC::Decoder &) ASSERTION FAILED: resources->contains(requestUrl) /Volumes/Data/slave/mojave-debug/build/Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp(92) : void WebKit::PrefetchCache::clearExpiredEntries()
https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r244422%20(2348)/http/tests/cache/memory-cache-pruning-crash-log.txt
Shawn Roberts
Comment 144
2019-04-18 09:45:57 PDT
rolled out in
http://trac.webkit.org/changeset/244424/webkit
Following up causing more assertion failures in Mac WK2 Debug builds.
Shawn Roberts
Comment 145
2019-04-18 12:42:41 PDT
Sorry, linked wrong revision. Rolled out in
https://trac.webkit.org/changeset/244428/webkit
Rob Buis
Comment 146
2019-04-19 09:46:13 PDT
Created
attachment 367804
[details]
Patch
Darin Adler
Comment 147
2019-04-19 10:01:10 PDT
What’s different about the new patch? I would review but I don’t know what to look for.
Rob Buis
Comment 148
2019-04-19 10:34:33 PDT
Comment on
attachment 367804
[details]
Patch Hi Darin, The only change is that PrefetchCache::take now removes entries from both sessionPrefetchMap and m_sessionExpirationList. Before only the sessionPrefetchMap entry was removed, so clearExpiredEntries still would process the m_sessionExpirationList and hit the resources->contains(requestUrl) ASSERT.
Darin Adler
Comment 149
2019-04-19 13:38:41 PDT
Comment on
attachment 367804
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367804&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:61 > + return std::get<0>(tuple) == sessionID && std::get<1>(tuple) == url;
I think it’s more elegant to write it this way: return tuple == std::tie(sessionID, url); Unless that doesn’t work for some reason?
Darin Adler
Comment 150
2019-04-19 13:39:35 PDT
Comment on
attachment 367804
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367804&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:30 > +#include <wtf/NeverDestroyed.h>
Really surprised this include is needed.
Alex Christensen
Comment 151
2019-04-19 16:44:25 PDT
Comment on
attachment 367804
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367804&action=review
> Source/WebCore/loader/ResourceLoadInfo.h:45 > + Prefetch = 0x0400,
This does not need to be added. If I am correct, these "Prefetch" loads are just Documents, right?
> Source/WebCore/loader/ResourceLoader.cpp:345 > + ASSERT(m_resourceType != ResourceType::Invalid || m_resourceType == ResourceType::Prefetch);
This is redundant. If the resource type is equal to Prefetch, then it is also not equal to Invalid. Why did you add this?
> Source/WebCore/loader/ResourceLoader.cpp:356 > + if (!redirectResponse.isNull() && frameLoader() && m_resourceType != ResourceType::Prefetch) {
This adds a way to bypass WKContentRuleLists entirely with prefetch. We do not want to do this.
Rob Buis
Comment 152
2019-04-20 10:12:15 PDT
Created
attachment 367891
[details]
Patch
Rob Buis
Comment 153
2019-04-22 02:59:30 PDT
Created
attachment 367932
[details]
Patch
Rob Buis
Comment 154
2019-04-22 09:01:29 PDT
Comment on
attachment 367804
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367804&action=review
>> Source/WebCore/loader/ResourceLoadInfo.h:45 >> + Prefetch = 0x0400, > > This does not need to be added. If I am correct, these "Prefetch" loads are just Documents, right?
More or less. I am not sure if you can navigate to a (document) image, I guess you can, but I think the only useful thing to navigate to is a Document, so I removed the Prefetch value.
>> Source/WebCore/loader/ResourceLoader.cpp:345 >> + ASSERT(m_resourceType != ResourceType::Invalid || m_resourceType == ResourceType::Prefetch); > > This is redundant. If the resource type is equal to Prefetch, then it is also not equal to Invalid. Why did you add this?
I did hit this at some point, but now removed since Prefetch value is gone.
>> Source/WebCore/loader/ResourceLoader.cpp:356 >> + if (!redirectResponse.isNull() && frameLoader() && m_resourceType != ResourceType::Prefetch) { > > This adds a way to bypass WKContentRuleLists entirely with prefetch. We do not want to do this.
Right, removed.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:30 >> +#include <wtf/NeverDestroyed.h> > > Really surprised this include is needed.
You are right, not needed. I think this stems from the initial singleton design.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:61 >> + return std::get<0>(tuple) == sessionID && std::get<1>(tuple) == url; > > I think it’s more elegant to write it this way: > > return tuple == std::tie(sessionID, url); > > Unless that doesn’t work for some reason?
This did not seem to work for me, I left as-is. I did not check the comparison operator implementation here but I think it has problems with the third tuple item.
Alex Christensen
Comment 155
2019-04-22 10:40:42 PDT
Comment on
attachment 367932
[details]
Patch Please include a test that blocks the prefetch load with a rule that has resource-type to make sure we are not introducing a way to bypass content rule lists. You can find similar tests in http/tests/contentextensions
Rob Buis
Comment 156
2019-04-22 23:48:38 PDT
Created
attachment 368016
[details]
Patch
Rob Buis
Comment 157
2019-04-23 05:14:15 PDT
(In reply to Alex Christensen from
comment #155
)
> Comment on
attachment 367932
[details]
> Patch > > Please include a test that blocks the prefetch load with a rule that has > resource-type to make sure we are not introducing a way to bypass content > rule lists. You can find similar tests in http/tests/contentextensions
Done.
Alex Christensen
Comment 158
2019-04-23 07:55:46 PDT
Comment on
attachment 368016
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=368016&action=review
> Source/WebKit/NetworkProcess/NetworkProcess.h:560 > + PrefetchCache m_prefetchCache;
This should probably live on the NetworkSession. We don't want private browsing to share state with non-private browsing.
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:50 > + RefPtr<WebCore::SharedBuffer> releaseBuffer() { return buffer.releaseNonNull(); }
If we're really releasing non null, then this could be a Ref.
> LayoutTests/http/tests/contentextensions/prefetch-blocked.html.json:7 > + "url-filter": "should-not-load"
This trigger should have "resource-type": "document"
Rob Buis
Comment 159
2019-04-24 08:00:57 PDT
Created
attachment 368124
[details]
Patch
EWS Watchlist
Comment 160
2019-04-24 09:10:49 PDT
Comment on
attachment 368124
[details]
Patch
Attachment 368124
[details]
did not pass mac-ews (mac): Output:
https://webkit-queues.webkit.org/results/11983393
New failing tests: http/tests/cache/link-prefetch-main-resource.html http/tests/cache/link-prefetch-main-resource-iframe.html
EWS Watchlist
Comment 161
2019-04-24 09:10:53 PDT
Created
attachment 368129
[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
EWS Watchlist
Comment 162
2019-04-24 09:52:13 PDT
Comment on
attachment 368124
[details]
Patch
Attachment 368124
[details]
did not pass mac-debug-ews (mac): Output:
https://webkit-queues.webkit.org/results/11983481
New failing tests: http/tests/cache/link-prefetch-main-resource.html http/tests/cache/link-prefetch-main-resource-iframe.html
EWS Watchlist
Comment 163
2019-04-24 09:52:16 PDT
Created
attachment 368134
[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
EWS Watchlist
Comment 164
2019-04-24 10:00:24 PDT
Comment on
attachment 368124
[details]
Patch
Attachment 368124
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/11983567
New failing tests: editing/pasteboard/5761530-1.html imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-abort.https.html
EWS Watchlist
Comment 165
2019-04-24 10:00:27 PDT
Created
attachment 368136
[details]
Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Rob Buis
Comment 166
2019-04-24 10:09:05 PDT
Created
attachment 368137
[details]
Patch
Rob Buis
Comment 167
2019-04-24 13:21:12 PDT
Comment on
attachment 368016
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=368016&action=review
>> Source/WebKit/NetworkProcess/NetworkProcess.h:560 >> + PrefetchCache m_prefetchCache; > > This should probably live on the NetworkSession. We don't want private browsing to share state with non-private browsing.
Done.
>> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:50 >> + RefPtr<WebCore::SharedBuffer> releaseBuffer() { return buffer.releaseNonNull(); } > > If we're really releasing non null, then this could be a Ref.
Done.
>> LayoutTests/http/tests/contentextensions/prefetch-blocked.html.json:7 >> + "url-filter": "should-not-load" > > This trigger should have "resource-type": "document"
Done.
youenn fablet
Comment 168
2019-04-25 13:42:02 PDT
Comment on
attachment 368137
[details]
Patch Some additional comments. View in context:
https://bugs.webkit.org/attachment.cgi?id=368137&action=review
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:214 > + if (isMainFrameLoad() && m_parameters.options.mode == FetchOptions::Mode::Navigate) {
We could simplify to isMainFrameLoad() here. If need be, we could also ASSERT(m_parameters.options.mode == FetchOptions::Mode::Navigate).
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1127 > +bool NetworkResourceLoader::isCrossOriginPrefetch(const ResourceRequest& request) const
We could simplify by removing request parameter and use originalRequest() here since prefetch -> redirect mode is manual. Do we have tests for prefetch redirections? We should be able to store in the prefetch cache these redirections.
> LayoutTests/http/tests/contentextensions/prefetch-blocked.html:3 > +<link rel="prefetch" href="resources/should-not-load.html">
Can we call testRunner.dumpAsText()?
Rob Buis
Comment 169
2019-04-26 01:02:41 PDT
Created
attachment 368311
[details]
Patch
EWS Watchlist
Comment 170
2019-04-26 02:47:27 PDT
Comment on
attachment 368311
[details]
Patch
Attachment 368311
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
https://webkit-queues.webkit.org/results/12003526
New failing tests: fast/css/first-letter-anonymous-block-crash.html
EWS Watchlist
Comment 171
2019-04-26 02:47:31 PDT
Created
attachment 368313
[details]
Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Rob Buis
Comment 172
2019-04-26 08:14:09 PDT
Created
attachment 368317
[details]
Patch
Rob Buis
Comment 173
2019-04-26 09:56:05 PDT
Created
attachment 368326
[details]
Patch
Rob Buis
Comment 174
2019-04-26 12:33:54 PDT
Created
attachment 368343
[details]
Patch
Rob Buis
Comment 175
2019-04-26 13:43:45 PDT
Comment on
attachment 368137
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=368137&action=review
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:214 >> + if (isMainFrameLoad() && m_parameters.options.mode == FetchOptions::Mode::Navigate) { > > We could simplify to isMainFrameLoad() here. > If need be, we could also ASSERT(m_parameters.options.mode == FetchOptions::Mode::Navigate).
Done.
>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:1127 >> +bool NetworkResourceLoader::isCrossOriginPrefetch(const ResourceRequest& request) const > > We could simplify by removing request parameter and use originalRequest() here since prefetch -> redirect mode is manual. > Do we have tests for prefetch redirections? > We should be able to store in the prefetch cache these redirections.
Done. Except we do not have tests for prefetch redirections yet. Do you have suggestions how to test prefetch redirections?
>> LayoutTests/http/tests/contentextensions/prefetch-blocked.html:3 >> +<link rel="prefetch" href="resources/should-not-load.html"> > > Can we call testRunner.dumpAsText()?
Done.
youenn fablet
Comment 176
2019-04-26 15:41:48 PDT
> > We could simplify by removing request parameter and use originalRequest() here since prefetch -> redirect mode is manual. > > Do we have tests for prefetch redirections? > > We should be able to store in the prefetch cache these redirections. > > Done. Except we do not have tests for prefetch redirections yet. Do you have > suggestions how to test prefetch redirections?
To be clear, it is fine to leave the support of storing prefetch redirections to a follow-up patch.This can be tested as follows: - Write a script that triggers a redirection for the first load and returns "FAIL" for any subsequent load. Script state might be stored using WPT/stash and cleared on demand. - Write a test prefetching the new script. Redirection will be stored. - Navigate to the script. redirection should be followed if prefetch is hit. Otherwise, the frame will show "FAIL" Another test that might be interesting to have is prefetched resources that are not cacheable or have a very short expiracy time.
Rob Buis
Comment 177
2019-04-29 08:58:49 PDT
(In reply to youenn fablet from
comment #176
)
> > > We could simplify by removing request parameter and use originalRequest() here since prefetch -> redirect mode is manual. > > > Do we have tests for prefetch redirections? > > > We should be able to store in the prefetch cache these redirections. > > > > Done. Except we do not have tests for prefetch redirections yet. Do you have > > suggestions how to test prefetch redirections? > > To be clear, it is fine to leave the support of storing prefetch > redirections to a follow-up patch.This can be tested as follows: > - Write a script that triggers a redirection for the first load and returns > "FAIL" for any subsequent load. Script state might be stored using WPT/stash > and cleared on demand. > - Write a test prefetching the new script. Redirection will be stored. > - Navigate to the script. redirection should be followed if prefetch is hit. > Otherwise, the frame will show "FAIL" > > Another test that might be interesting to have is prefetched resources that > are not cacheable or have a very short expiracy time.
Thanks for the decriptions, I have opened
https://bugs.webkit.org/show_bug.cgi?id=197371
for it.
Rob Buis
Comment 178
2019-04-30 10:41:56 PDT
(In reply to Rob Buis from
comment #177
)
> > To be clear, it is fine to leave the support of storing prefetch > > redirections to a follow-up patch.This can be tested as follows: > > - Write a script that triggers a redirection for the first load and returns > > "FAIL" for any subsequent load. Script state might be stored using WPT/stash > > and cleared on demand. > > - Write a test prefetching the new script. Redirection will be stored. > > - Navigate to the script. redirection should be followed if prefetch is hit. > > Otherwise, the frame will show "FAIL" > > > > Another test that might be interesting to have is prefetched resources that > > are not cacheable or have a very short expiracy time. > > Thanks for the decriptions, I have opened >
https://bugs.webkit.org/show_bug.cgi?id=197371
for it.
To be clear, I meant a test for the redirect, not the short expiracy test. So the mentioned bug now has tat test, and I have a local fix to make it pass, but it is easier if this patch goes in first, to make sure the dependent 197371 patch builds.
youenn fablet
Comment 179
2019-05-07 13:42:01 PDT
Comment on
attachment 368343
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=368343&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:50 > + Ref<WebCore::SharedBuffer> releaseBuffer() { return buffer.releaseNonNull(); }
I do not think we can guarantee buffer will always be null. We might just need to WTFMove(buffer) when storing it in network cache.
youenn fablet
Comment 180
2019-05-07 13:42:07 PDT
Comment on
attachment 368343
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=368343&action=review
> Source/WebKit/NetworkProcess/cache/PrefetchCache.h:50 > + Ref<WebCore::SharedBuffer> releaseBuffer() { return buffer.releaseNonNull(); }
I do not think we can guarantee buffer will always be null. We might just need to WTFMove(buffer) when storing it in network cache.
WebKit Commit Bot
Comment 181
2019-05-08 00:01:10 PDT
Comment on
attachment 368343
[details]
Patch Rejecting
attachment 368343
[details]
from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 368343, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as
commit-queue@webkit.org
... Fetching:
https://bugs.webkit.org/attachment.cgi?id=368343&action=edit
Fetching:
https://bugs.webkit.org/show_bug.cgi?id=195623
&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Processing patch 368343 from
bug 195623
. Fetching:
https://bugs.webkit.org/attachment.cgi?id=368343
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Youenn Fablet']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Parsed 25 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/loader/LinkLoader.cpp patching file Source/WebCore/loader/ResourceLoadInfo.cpp patching file Source/WebKit/NetworkProcess/NetworkProcess.cpp patching file Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp Hunk #4 succeeded at 809 (offset -6 lines). Hunk #5 succeeded at 1144 (offset -6 lines). patching file Source/WebKit/NetworkProcess/NetworkResourceLoader.h patching file Source/WebKit/NetworkProcess/NetworkSession.h Hunk #2 succeeded at 94 (offset 1 line). Hunk #3 succeeded at 113 (offset 1 line). patching file Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp patching file Source/WebKit/NetworkProcess/cache/PrefetchCache.h patching file Source/WebKit/Shared/WebPreferences.yaml Hunk #1 succeeded at 1665 (offset 15 lines). patching file Source/WebKit/Sources.txt patching file Source/WebKit/WebKit.xcodeproj/project.pbxproj Hunk #2 succeeded at 3997 (offset 2 lines). patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/http/tests/cache/link-prefetch-main-resource-expected.txt patching file LayoutTests/http/tests/cache/link-prefetch-main-resource-iframe-expected.txt patching file LayoutTests/http/tests/cache/link-prefetch-main-resource-iframe.html patching file LayoutTests/http/tests/cache/link-prefetch-main-resource.html patching file LayoutTests/http/tests/cache/resources/prefetched-main-resource-iframe.php patching file LayoutTests/http/tests/cache/resources/prefetched-main-resource.php patching file LayoutTests/http/tests/contentextensions/prefetch-blocked-expected.txt patching file LayoutTests/http/tests/contentextensions/prefetch-blocked.html patching file LayoutTests/http/tests/contentextensions/prefetch-blocked.html.json patching file LayoutTests/platform/mac-wk1/TestExpectations Hunk #1 succeeded at 702 (offset -3 lines). patching file LayoutTests/platform/win/TestExpectations Hunk #1 FAILED at 4335. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/win/TestExpectations.rej Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Youenn Fablet']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output:
https://webkit-queues.webkit.org/results/12130900
Rob Buis
Comment 182
2019-05-08 04:10:02 PDT
Created
attachment 369372
[details]
Patch
WebKit Commit Bot
Comment 183
2019-05-08 05:40:17 PDT
Comment on
attachment 369372
[details]
Patch Clearing flags on attachment: 369372 Committed
r245053
: <
https://trac.webkit.org/changeset/245053
>
WebKit Commit Bot
Comment 184
2019-05-08 05:40:21 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug