WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
219628
Add max age for a root to be reused.
https://bugs.webkit.org/show_bug.cgi?id=219628
Summary
Add max age for a root to be reused.
dewei_zhu
Reported
2020-12-07 21:20:48 PST
Add max age for a root to be reused.
Attachments
Patch
(15.47 KB, patch)
2020-12-07 21:33 PST
,
dewei_zhu
rniwa
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
dewei_zhu
Comment 1
2020-12-07 21:33:14 PST
Created
attachment 415611
[details]
Patch
Ryosuke Niwa
Comment 2
2020-12-07 21:45:44 PST
Comment on
attachment 415611
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=415611&action=review
> Websites/perf.webkit.org/public/include/manifest-generator.php:53 > + 'maxReuseRootAgeInDays' => config('maxReuseRootAgeInDays'),
max*RootReuse*AgeInDays
> Websites/perf.webkit.org/public/v3/models/build-request.js:100 > + const earliestAllowRootCreationDate = rawManifest.maxReuseRootAgeInDays ? > + Date.now() - rawManifest.maxReuseRootAgeInDays * 24 * 3600 * 1000 : 0;
I'd call this earliestRootCreatingTimeForReuse
> Websites/perf.webkit.org/public/v3/models/commit-set.js:77 > - areAllRootsAvailable() > + areAllRootsAvailableAndNewerThan(earliestCreationDate)
I don't think it's necessary to call it "andNewerThan". Also, I'd call the new argument earliestCreationTime instead.
> Websites/perf.webkit.org/public/v3/models/manifest.js:25 > static fetch()
Why don't we make this async as well?
> Websites/perf.webkit.org/public/v3/models/manifest.js:34 > + return await RemoteAPI.getJSON('/data/manifest.json');
Neat.
> Websites/perf.webkit.org/public/v3/models/manifest.js:35 > + } catch(error) {
We shouldn't swallow all errors though. Just 404. We should probably add a test for that.
> Websites/perf.webkit.org/unit-tests/build-request-tests.js:470 > + it('should not reuse a root that older than "maxReuseRootAge"', async () => {
that *is* older than.
dewei_zhu
Comment 3
2020-12-09 15:13:07 PST
Comment on
attachment 415611
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=415611&action=review
>> Websites/perf.webkit.org/public/v3/models/manifest.js:35 >> + } catch(error) { > > We shouldn't swallow all errors though. Just 404. > We should probably add a test for that.
Added in the change.
dewei_zhu
Comment 4
2020-12-09 15:14:15 PST
Landed in
r270607
.
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