WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
216371
Some WebAudio tests give different output on different machines
https://bugs.webkit.org/show_bug.cgi?id=216371
Summary
Some WebAudio tests give different output on different machines
Chris Dumez
Reported
2020-09-10 09:50:45 PDT
Some WebAudio tests give different output on different machines, because they rely on the hardware's preferred sample rate.
Attachments
Patch
(34.03 KB, patch)
2020-09-10 09:56 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(25.44 KB, patch)
2020-09-14 09:18 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2020-09-10 09:56:11 PDT
Created
attachment 408453
[details]
Patch
Alex Christensen
Comment 2
2020-09-10 10:10:03 PDT
Comment on
attachment 408453
[details]
Patch Could you pass it through the WebPageCreationParameters instead of making new bundle SPI?
Chris Dumez
Comment 3
2020-09-10 10:13:32 PDT
(In reply to Alex Christensen from
comment #2
)
> Comment on
attachment 408453
[details]
> Patch > > Could you pass it through the WebPageCreationParameters instead of making > new bundle SPI?
I was trying to reduce the amount of plumbing necessary. Also, this hardwareSampleRate() is a global static so I did not really want this to be per page.
Alex Christensen
Comment 4
2020-09-10 10:17:39 PDT
Comment on
attachment 408453
[details]
Patch Why do you pass a WKBundleRef then? I think we should not be expanding the bundle SPI, even for testing, if we can get away with not expanding it. In this case we can.
Chris Dumez
Comment 5
2020-09-10 10:23:47 PDT
(In reply to Alex Christensen from
comment #4
)
> Comment on
attachment 408453
[details]
> Patch > > Why do you pass a WKBundleRef then?
> I think we should not be expanding the bundle SPI, even for testing, if we > can get away with not expanding it. In this case we can.
Can you clarify your proposal? You want the SPI to be on the WKWebView even though the sample rate is process-wide? Also, how about when the view swaps process? I would need to add logic to forward the sample rate to the new process? Doing it like you suggest adds more code and complexity.
Alex Christensen
Comment 6
2020-09-10 10:41:35 PDT
Comment on
attachment 408453
[details]
Patch I think we should make a WKWebViewConfigurationPrivateForTesting.h like we have WKWebViewPrivateForTesting.h, add an override there, then copy it to an Optional on WebPageCreationParameters, then in the WebPage constructor if it's not nullopt we set it globally in that process. When we process swap, a new WebPageCreationParameters is already sent to the new process. It's not perfect, but it's good enough for test-only SPI. If you really think that's not a good direction and would prefer increasing our dependence on the bundle, r=me
Chris Dumez
Comment 7
2020-09-10 11:20:41 PDT
Committed
r266842
: <
https://trac.webkit.org/changeset/266842
>
Radar WebKit Bug Importer
Comment 8
2020-09-10 11:21:18 PDT
<
rdar://problem/68653909
>
Alex Christensen
Comment 9
2020-09-10 11:29:07 PDT
I think the best solution would have been to modify the web platform tests to print out a deterministic output.
Chris Dumez
Comment 10
2020-09-10 15:57:56 PDT
Reverted
r266842
and
r266883
for reason: Causes some assertions to be hit in debug builds Committed
r266895
: <
https://trac.webkit.org/changeset/266895
>
Chris Dumez
Comment 11
2020-09-14 09:18:24 PDT
Created
attachment 408710
[details]
Patch
Chris Dumez
Comment 12
2020-09-14 09:18:45 PDT
(In reply to Chris Dumez from
comment #11
)
> Created
attachment 408710
[details]
> Patch
New approach so requesting review again.
Alex Christensen
Comment 13
2020-09-14 09:20:48 PDT
Comment on
attachment 408710
[details]
Patch For some reason I don't understand, I think this is better than bundle SPI. I think it would be even better to modify the test to have different output (just PASS/FAIL instead of printing audio sample rates) but this is fine too.
Chris Dumez
Comment 14
2020-09-14 10:01:00 PDT
Comment on
attachment 408710
[details]
Patch Clearing flags on attachment: 408710 Committed
r267018
: <
https://trac.webkit.org/changeset/267018
>
Chris Dumez
Comment 15
2020-09-14 10:01:03 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