Bug 170739 - Add a preference for the media resource buffering time
Summary: Add a preference for the media resource buffering time
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-11 10:50 PDT by Alex Christensen
Modified: 2017-04-11 14:35 PDT (History)
1 user (show)

See Also:

Attachments
Patch (16.50 KB, patch)
2017-04-11 10:54 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (16.69 KB, patch)
2017-04-11 13:09 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (7.09 KB, patch)
2017-04-11 14:34 PDT, Alex Christensen 		achristensen: review-
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2017-04-11 10:50:43 PDT 
Add a preference for the media resource buffering time
Comment 1 Alex Christensen 2017-04-11 10:54:15 PDT 
Created attachment 306833 [details]
Patch
Comment 2 Alex Christensen 2017-04-11 11:12:42 PDT 
rdar://problem/31485485
Comment 3 Sam Weinig 2017-04-11 12:04:47 PDT 
Comment on attachment 306833 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKPreferences.mm:590
> +- (double)_mediaResourceMaximumBufferingTime

I think the suffix Duration, rather than Time, would be better.

> Source/WebKit2/UIProcess/Cocoa/WebProcessPoolCocoa.mm:251
> +    parameters.mediaResourceMaximumBufferingTime = Seconds(m_defaultPageGroup->preferences().mediaResourceMaximumBufferingTime());

Seems weird/wrong to have webpreferences affect WebProcess creation, since a single web process can have web pages with different webpreferences.  It also looks like you aren't handling the preference changing after the process is created.
Comment 4 Alex Christensen 2017-04-11 12:06:55 PDT 
I agree that it's icky, but this is a consequence of the WebResourceLoader being process global in the WebProcess.  And it's only temporary, so I'll remove it soon.  I'll change the suffix, though.
Comment 5 Alex Christensen 2017-04-11 13:09:44 PDT 
Created attachment 306843 [details]
Patch
Comment 6 Alex Christensen 2017-04-11 14:34:50 PDT 
Created attachment 306851 [details]
Patch
Comment 7 Alex Christensen 2017-04-11 14:35:18 PDT 
Comment on attachment 306851 [details]
Patch

This patch is a step in the wrong direction, but it will allow me to do local testing easier.