Seeing the following assertion failure on Big Sur Debug bots, causing tests to exit early: SHOULD NEVER BE REACHED /Volumes/Data/worker/bigsur-debug/build/Source/WebCore/platform/network/cf/ResourceRequestCFNet.h(62) : WebCore::ResourceLoadPriority WebCore::toResourceLoadPriority(CFURLRequestPriority) 1 0x142581769 WTFCrash 2 0x12602143b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x128207fca WebCore::toResourceLoadPriority(long) 4 0x12820788b WebCore::ResourceRequest::doUpdateResourceRequest() 5 0x12a7c2e67 WebCore::ResourceRequestBase::updateResourceRequest(WebCore::HTTPBodyUpdatePolicy) const 6 0x12a7c2a74 WebCore::ResourceRequestBase::setPriority(WebCore::ResourceLoadPriority) 7 0x12b51da8d WebCore::WebCoreAVFResourceLoader::startLoading() 8 0x126423fee WebCore::MediaPlayerPrivateAVFoundationObjC::shouldWaitForLoadingOfResource(AVAssetResourceLoadingRequest*) 9 0x12646e1c0 -[WebCoreAVFLoaderDelegate resourceLoader:shouldWaitForLoadingOfRequestedResource:]::$_15::operator()()::'lambda'()::operator()() const 10 0x12646e06e WTF::Detail::CallableWrapper<-[WebCoreAVFLoaderDelegate resourceLoader:shouldWaitForLoadingOfRequestedResource:]::$_15::operator()()::'lambda'(), void>::call() 11 0x126036422 WTF::Function<void ()>::operator()() const 12 0x1263f1765 WebCore::GenericTaskQueue<WebCore::Timer>::enqueueTask(WTF::Function<void ()>&&)::'lambda'()::operator()() const 13 0x1263f159e WTF::Detail::CallableWrapper<WebCore::GenericTaskQueue<WebCore::Timer>::enqueueTask(WTF::Function<void ()>&&)::'lambda'(), void>::call() 14 0x126036422 WTF::Function<void ()>::operator()() const 15 0x12a1fc82f WebCore::TaskDispatcher<WebCore::Timer>::dispatchOneTask() 16 0x12a1fc505 WebCore::TaskDispatcher<WebCore::Timer>::sharedTimerFired() 17 0x12a203e51 WebCore::TaskDispatcher<WebCore::Timer>::sharedTimer()::$_2::operator()() const 18 0x12a203e0e WTF::Detail::CallableWrapper<WebCore::TaskDispatcher<WebCore::Timer>::sharedTimer()::$_2, void>::call() 19 0x126036422 WTF::Function<void ()>::operator()() const 20 0x12607974e WebCore::Timer::fired() 21 0x12a25ffa4 WebCore::ThreadTimers::sharedTimerFiredInternal() 22 0x12a26a481 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const 23 0x12a26a42e WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, void>::call() 24 0x126036422 WTF::Function<void ()>::operator()() const 25 0x12a217a5b WebCore::MainThreadSharedTimer::fired() 26 0x12a2e8176 WebCore::timerFired(__CFRunLoopTimer*, void*) 27 0x7fff2047990d __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ 28 0x7fff204793e8 __CFRunLoopDoTimer 29 0x7fff20478f42 __CFRunLoopDoTimers 30 0x7fff2045f57f __CFRunLoopRun 31 0x7fff2045e6ce CFRunLoopRunSpecific https://build.webkit.org/results/Apple-BigSur-Debug-WK2-Tests/r274171%20(608)/results.html
<rdar://problem/75236575>
This assert was added with https://trac.webkit.org/changeset/274161/webkit
Created attachment 422766 [details] Patch
Committed r274180 (235099@main): <https://commits.webkit.org/235099@main>
*** Bug 223003 has been marked as a duplicate of this bug. ***
(In reply to Ben Nham from comment #4) > Committed r274180 (235099@main): <https://commits.webkit.org/235099@main> Is VeryLow the priority we really want when we don't have a priority set?
Comment on attachment 422766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422766&action=review > Source/WebCore/platform/network/cf/ResourceRequestCFNet.h:53 > return ResourceLoadPriority::VeryLow; Is it OK correct to translate this -1 into VeryLow? Why not Medium? I don’t understand the repercussions.
(In reply to Darin Adler from comment #8) > Comment on attachment 422766 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=422766&action=review > > > Source/WebCore/platform/network/cf/ResourceRequestCFNet.h:53 > > return ResourceLoadPriority::VeryLow; > > Is it OK correct to translate this -1 into VeryLow? Why not Medium? I don’t > understand the repercussions. Ahah that’s what I asked as well :)
(In reply to Darin Adler from comment #8) > Comment on attachment 422766 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=422766&action=review > > > Source/WebCore/platform/network/cf/ResourceRequestCFNet.h:53 > > return ResourceLoadPriority::VeryLow; > > Is it OK correct to translate this -1 into VeryLow? Why not Medium? I don’t > understand the repercussions. (In reply to Chris Dumez from comment #9) > (In reply to Darin Adler from comment #8) > > Comment on attachment 422766 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=422766&action=review > > > > > Source/WebCore/platform/network/cf/ResourceRequestCFNet.h:53 > > > return ResourceLoadPriority::VeryLow; > > > > Is it OK correct to translate this -1 into VeryLow? Why not Medium? I don’t > > understand the repercussions. > > Ahah that’s what I asked as well :) From a regression point of view, mapping -1 to VeryLow is fine since that is exactly what we were doing before: https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/network/cf/ResourceRequestCFNet.h?rev=274160#L52 and the overall mapping is now exactly the same as it was before I introduced the FIXME in r252431: https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/network/cf/ResourceRequestCFNet.h?rev=252430#L46 As for what the proper default request priority is, I'm not really sure. It probably should be ResourceLoadPriority::Low based on the fact that this is the default used in both CachedResource::defaultPriorityForResourceType and ResourceRequestBase::m_priority. In practice I don't think it would make any perf difference since the call sites I looked at all set an explicit priority anyway. But I can make a separate cleanup patch tomorrow that adds a ResourceLoadPriority::Default and return that when the CF request priority is unset.
Now that I think about it, the existing mapping of CFURLRequestPriority -1 to ResourceLoadPriority::VeryLow makes sense, so I don't think I'm going to change that. The reason is that if you don't set a request priority on the CFURLRequest (meaning CFURLRequestGetRequestPriority returns -1), it actually gets treated as if you had set a priority of 0 (see HTTPConnectionCacheEntry::_prepareNewRequest). So the fact that both -1 and 0 both map to the same value of VeryLow reflects actual behavior. I could add a new enum value ResourceLoadPriority::Unspecified for this -1 case, which would probably be the best idea if I was starting from scratch. But it doesn't seem like a worthwhile refactoring given that this current mapping of -1 to VeryLow has been the same for years (until I accidentally removed it) and reflects reality.