Bug 182488 – Use downcast in createLinkPreloadResourceClient

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

This will make the code more robust.

[youenn fablet]

Created attachment 333083 [details]
Patch

[EWS Watchlist]

Comment on attachment 333083 [details]
Patch

Attachment 333083 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6366488

New failing tests:
media/modern-media-controls/tracks-support/tracks-support-show-panel-after-dragging-controls.html

[EWS Watchlist]

Created attachment 333087 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[youenn fablet]

Error is unrelated.

[WebKit Commit Bot]

Comment on attachment 333083 [details]
Patch

Clearing flags on attachment: 333083

Committed r228103: <https://trac.webkit.org/changeset/228103>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37242289>

[Matt Lewis]

Reverted r228103 for reason:

This caused multiple tests to crash.

Committed r228117: <https://trac.webkit.org/changeset/228117>

[Matt Lewis]

The test that were crashing were: 
  http/tests/preload/download_resources.html [ Crash ]
  http/tests/preload/onerror_event.html [ Crash ]
  http/tests/preload/onload_event.html [ Crash ]
  http/tests/preload/single_download_preload.html [ Crash ]
  http/wpt/preload/type-attribute.html [ Crash ]


build:
https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/r228106%20(2536)/results.html
https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/builds/2536

crash backtrace:
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   JavaScriptCore                	0x0000000111a427f4 WTFCrash + 36
1   JavaScriptCore                	0x0000000111a42809 WTFCrashWithSecurityImplication + 9
2   com.apple.WebCore             	0x0000000116241a01 WTF::match_constness<WebCore::CachedResource, WebCore::CachedRawResource>::type& WTF::downcast<WebCore::CachedRawResource, WebCore::CachedResource>(WebCore::CachedResource&) + 65
3   com.apple.WebCore             	0x000000011623e5f3 WebCore::createLinkPreloadResourceClient(WebCore::CachedResource&, WebCore::LinkLoader&) + 3715
4   com.apple.WebCore             	0x000000011623d11c WebCore::LinkLoader::preloadIfNeeded(WebCore::LinkRelAttribute const&, WebCore::URL const&, WebCore::Document&, WTF::String const&, WTF::String const&, WTF::String const&, WTF::String const&, WebCore::LinkLoader*) + 2188
5   com.apple.WebCore             	0x000000011623ef87 WebCore::LinkLoader::loadLink(WebCore::LinkRelAttribute const&, WebCore::URL const&, WTF::String const&, WTF::String const&, WTF::String const&, WTF::String const&, WebCore::Document&) + 1159
6   com.apple.WebCore             	0x0000000115ebd315 WebCore::HTMLLinkElement::process() + 533
7   com.apple.WebCore             	0x0000000115ebea85 WebCore::HTMLLinkElement::didFinishInsertingNode() + 21
8   com.apple.WebCore             	0x0000000115af10e8 void WebCore::executeNodeInsertionWithScriptAssertion<WebCore::ContainerNode::parserAppendChild(WebCore::Node&)::$_5>(WebCore::ContainerNode&, WebCore::Node&, WebCore::ContainerNode::ChildChangeSource, WebCore::ReplacedAllChildren, WebCore::ContainerNode::parserAppendChild(WebCore::Node&)::$_5) + 680
9   com.apple.WebCore             	0x0000000115aed2ed WebCore::ContainerNode::parserAppendChild(WebCore::Node&) + 285
10  com.apple.WebCore             	0x0000000115fdcb88 WebCore::insert(WebCore::HTMLConstructionSiteTask&) + 344
11  com.apple.WebCore             	0x0000000115fdc5eb WebCore::executeInsertTask(WebCore::HTMLConstructionSiteTask&) + 75
12  com.apple.WebCore             	0x0000000115fc8696 WebCore::executeTask(WebCore::HTMLConstructionSiteTask&) + 70
13  com.apple.WebCore             	0x0000000115fc8552 WebCore::HTMLConstructionSite::executeQueuedTasks() + 146
14  com.apple.WebCore             	0x0000000116006170 WebCore::HTMLTreeBuilder::constructTree(WebCore::AtomicHTMLToken&&) + 480
15  com.apple.WebCore             	0x0000000115fd1917 WebCore::HTMLDocumentParser::constructTreeFromHTMLToken(WebCore::HTMLTokenizer::TokenPtr&) + 151
16  com.apple.WebCore             	0x0000000115fd15e8 WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&) + 1336
17  com.apple.WebCore             	0x0000000115fcfec8 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) + 424
18  com.apple.WebCore             	0x0000000115fcfa3b WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode) + 171
19  com.apple.WebCore             	0x0000000115fd2be9 WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution() + 377
20  com.apple.WebCore             	0x0000000115fd2fee WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&) + 366
21  com.apple.WebCore             	0x0000000115fd304c non-virtual thunk to WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&) + 44
22  com.apple.WebCore             	0x0000000115c70e17 WebCore::PendingScript::notifyClientFinished() + 71
23  com.apple.WebCore             	0x0000000115c70e79 WebCore::PendingScript::notifyFinished(WebCore::LoadableScript&) + 25
24  com.apple.WebCore             	0x0000000115c23f89 WebCore::LoadableScript::notifyClientFinished() + 329
25  com.apple.WebCore             	0x0000000115c23d8f WebCore::LoadableClassicScript::notifyFinished(WebCore::CachedResource&) + 927
26  com.apple.WebCore             	0x0000000115c2400c non-virtual thunk to WebCore::LoadableClassicScript::notifyFinished(WebCore::CachedResource&) + 44
27  com.apple.WebCore             	0x00000001162e8a8d WebCore::CachedResource::checkNotify() + 125
28  com.apple.WebCore             	0x00000001162dbb51 WebCore::CachedResource::finishLoading(WebCore::SharedBuffer*) + 49
29  com.apple.WebCore             	0x000000011631699f WebCore::CachedScript::finishLoading(WebCore::SharedBuffer*) + 143
30  com.apple.WebCore             	0x00000001162826da WebCore::SubresourceLoader::didFinishLoading(WebCore::NetworkLoadMetrics const&) + 794
31  com.apple.WebKit              	0x0000000104295ccd WebKit::WebResourceLoader::didFinishResourceLoad(WebCore::NetworkLoadMetrics const&) + 141
32  com.apple.WebKit              	0x000000010429929a void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, 0ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>&&, std::__1::integer_sequence<unsigned long, 0ul>) + 154
33  com.apple.WebKit              	0x0000000104299100 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, std::__1::integer_sequence<unsigned long, 0ul> >(std::__1::tuple<WebCore::NetworkLoadMetrics>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&)) + 96
34  com.apple.WebKit              	0x0000000104298536 void IPC::handleMessage<Messages::WebResourceLoader::DidFinishResourceLoad, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&)) + 262
35  com.apple.WebKit              	0x0000000104297c3c WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&) + 636
36  com.apple.WebKit              	0x0000000103a1aff9 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 169
37  com.apple.WebKit              	0x00000001037dc7d3 IPC::Connection::dispatchMessage(IPC::Decoder&) + 51
38  com.apple.WebKit              	0x00000001037d23f8 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 712
39  com.apple.WebKit              	0x00000001037dcdda IPC::Connection::dispatchOneMessage() + 1530
40  com.apple.WebKit              	0x00000001037f4bfd IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() + 29
41  com.apple.WebKit              	0x00000001037f4b59 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() + 25
42  JavaScriptCore                	0x0000000111a5ea7b WTF::Function<void ()>::operator()() const + 139
43  JavaScriptCore                	0x0000000111aa22cd WTF::RunLoop::performWork() + 445
44  JavaScriptCore                	0x0000000111aa2a44 WTF::RunLoop::performWork(void*) + 36
45  com.apple.CoreFoundation      	0x0000000107d5d2b1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
46  com.apple.CoreFoundation      	0x0000000107dfcd31 __CFRunLoopDoSource0 + 81
47  com.apple.CoreFoundation      	0x0000000107d41c19 __CFRunLoopDoSources0 + 185
48  com.apple.CoreFoundation      	0x0000000107d411ff __CFRunLoopRun + 1279
49  com.apple.CoreFoundation      	0x0000000107d40a89 CFRunLoopRunSpecific + 409
50  com.apple.Foundation          	0x000000010309be5e -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 274
51  com.apple.Foundation          	0x000000010309bd39 -[NSRunLoop(NSRunLoop) run] + 76
52  libxpc.dylib                  	0x00000001098500d9 _xpc_objc_main + 460
53  libxpc.dylib                  	0x00000001098524cb xpc_main + 143
54  com.apple.WebKit.WebContent   	0x0000000102fefbfe main + 894
55  libdyld.dylib                 	0x00000001094fbd81 start + 1

[youenn fablet]

Created attachment 333119 [details]
Patch

[Antti Koivisto]

Comment on attachment 333119 [details]
Patch

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

> Source/WebCore/loader/LinkPreloadResourceClients.h:81
> +template<typename SpecificCachedResource>
> +class LinkPreloadCachedResourceClient final: public LinkPreloadResourceClient, CachedResourceClient {

Template seems like an overkill to cover two cases. How about just having two create() functions taking CachedScript and CachedTextTrack? The rest of the type can use CachedResource&.

[youenn fablet]

> > Source/WebCore/loader/LinkPreloadResourceClients.h:81
> > +template<typename SpecificCachedResource>
> > +class LinkPreloadCachedResourceClient final: public LinkPreloadResourceClient, CachedResourceClient {
> 
> Template seems like an overkill to cover two cases. How about just having
> two create() functions taking CachedScript and CachedTextTrack? The rest of
> the type can use CachedResource&.

I kind of agree with these two only.
I was thinking we could go with the following in the future:

template<typename SpecificCachedResource, typename SpecificCachedResourceClient>
class LinkPreloadGenericCachedResourceClient final: public LinkPreloadResourceClient, SpecificCachedResourceClient {
...
}

Then LinkPreloadImageResourceClient and LinkPreloadRawResourceClient would become something like: 
using LinkPreloadImageResourceClient = LinkPreloadGenericCachedResourceClient<CachedImage, CachedImageClient>;
using LinkPreloadRawResourceClient = LinkPreloadGenericCachedResourceClient<CachedRawResource, CachedRawResourceClient>;

wdyt?

[youenn fablet]

Created attachment 333222 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 333222 [details]
Patch for landing

Clearing flags on attachment: 333222

Committed r228201: <https://trac.webkit.org/changeset/228201>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ross Kirsling]

The new `create` method is missing an #if ENABLE(VIDEO_TRACK) guard. I've submitted a fix here:
https://bugs.webkit.org/show_bug.cgi?id=182585