Use ResourceLoader to load apache manifest
Created attachment 334001 [details] Patch
Created attachment 334003 [details] Patch
Created attachment 334005 [details] Patch
Comment on attachment 334005 [details] Patch Attachment 334005 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6530809 New failing tests: http/tests/appcache/auth.html
Created attachment 334012 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 334005 [details] Patch Attachment 334005 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6530807 New failing tests: http/tests/appcache/auth.html
Created attachment 334014 [details] Archive of layout-test-results from ews115 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 334044 [details] Patch
Comment on attachment 334044 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334044&action=review This is exciting! Do you want it reviewed? > Source/WebCore/loader/appcache/ApplicationCacheResourceLoader.cpp:14 > + * * Neither the name of Google Inc. nor the names of its This copyright should be the two-clause copyright used in new files now. > LayoutTests/http/tests/appcache/manifest-redirect-2-expected.txt:2 > +CONSOLE MESSAGE: Application Cache manifest could not be fetched, because a redirection was attempted. Is not supporting redirection in the manifest a new thing, or is this just a new message?
(In reply to Alex Christensen from comment #9) > Comment on attachment 334044 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334044&action=review > > This is exciting! Do you want it reviewed? > > > Source/WebCore/loader/appcache/ApplicationCacheResourceLoader.cpp:14 > > + * * Neither the name of Google Inc. nor the names of its > > This copyright should be the two-clause copyright used in new files now. Will fix. > > LayoutTests/http/tests/appcache/manifest-redirect-2-expected.txt:2 > > +CONSOLE MESSAGE: Application Cache manifest could not be fetched, because a redirection was attempted. > > Is not supporting redirection in the manifest a new thing, or is this just a > new message? It is part of preexisting code checks, in ApplicationCacheGroup::didReceiveManifestResponse (removed in this patch).
Created attachment 334053 [details] Patch
Created attachment 334054 [details] Patch
Comment on attachment 334054 [details] Patch Clearing flags on attachment: 334054 Committed r228575: <https://trac.webkit.org/changeset/228575>
All reviewed patches have been landed. Closing bug.
<rdar://problem/37616550>
Reverted r228575 for reason: Introduced LayoutTest crashes under GuardMalloc. Committed r228591: <https://trac.webkit.org/changeset/228591>
Comment on attachment 334054 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334054&action=review > Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:448 > + m_loader = ApplicationCacheResourceLoader::create(documentLoader.cachedResourceLoader(), WTFMove(request), [this] (auto&& resourceOrError) { I'll bet you need to protect something here to make it not UAF.
(In reply to Alex Christensen from comment #17) > Comment on attachment 334054 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334054&action=review > > > Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:448 > > + m_loader = ApplicationCacheResourceLoader::create(documentLoader.cachedResourceLoader(), WTFMove(request), [this] (auto&& resourceOrError) { > > I'll bet you need to protect something here to make it not UAF. I am not sure. In ApplicationCacheGroup destructor, we are stopping the loads, triggering the callback with an abort error.
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x000000010f1db231 WebCore::ApplicationCacheResourceLoader::cancel(WebCore::ApplicationCacheResourceLoader::Error) + 145 1 com.apple.WebCore 0x000000010e2887b4 WebCore::CachedRawResource::responseReceived(WebCore::ResourceResponse const&) + 308 2 com.apple.WebCore 0x000000010e28825a WebCore::SubresourceLoader::didReceiveResponse(WebCore::ResourceResponse const&) + 1130 3 com.apple.WebCore 0x000000010f1ca7a9 WebCore::ResourceLoader::didReceiveResponseAsync(WebCore::ResourceHandle*, WebCore::ResourceResponse&&, WTF::CompletionHandler<void ()>&&) + 57 4 com.apple.WebCore 0x000000010f40e91a WebCore::ResourceHandle::didReceiveResponse(WebCore::ResourceResponse&&, WTF::CompletionHandler<void ()>&&) + 346 5 com.apple.WebCore 0x000000010e8340f1 WTF::Function<void ()>::CallableWrapper<-[WebCoreResourceHandleAsOperationQueueDelegate connection:didReceiveResponse:]::$_4>::call() + 449 6 com.apple.JavaScriptCore 0x000000010ae2dd00 WTF::dispatchFunctionsFromMainThread() + 176 7 com.apple.Foundation 0x00007fff31ed7e75 __NSThreadPerformPerform + 334 8 com.apple.CoreFoundation 0x00007fff2fcf5611 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 9 com.apple.CoreFoundation 0x00007fff2fdacecc __CFRunLoopDoSource0 + 108 10 com.apple.CoreFoundation 0x00007fff2fcd86a0 __CFRunLoopDoSources0 + 208 11 com.apple.CoreFoundation 0x00007fff2fcd7b1d __CFRunLoopRun + 1293 12 com.apple.CoreFoundation 0x00007fff2fcd7377 CFRunLoopRunSpecific + 487 13 DumpRenderTree 0x000000010ac541b2 0x10ac41000 + 78258 14 DumpRenderTree 0x000000010ac53508 0x10ac41000 + 75016 15 DumpRenderTree 0x000000010ac54c8c 0x10ac41000 + 81036 16 libdyld.dylib 0x00007fff58581015 start + 1
Ok, so I probably need to protect 'this' in cancel. And there is apparently no guarantee that an ApplicationCacheGroup stays alive after didFailLoadingManifest/didFinishLoadingManifest, making the assertions I added bad.
(In reply to youenn fablet from comment #20) > Ok, so I probably need to protect 'this' in cancel. > And there is apparently no guarantee that an ApplicationCacheGroup stays > alive after didFailLoadingManifest/didFinishLoadingManifest, making the > assertions I added bad. You can validate by running tests in the http/tests/appcache folder with -g.
Created attachment 334387 [details] Changes to fix guard malloc crashes
Created attachment 334388 [details] Fixing guard malloc crashes
Comment on attachment 334388 [details] Fixing guard malloc crashes Rejecting attachment 334388 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 334388, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/6611674
Created attachment 334397 [details] Patch for landing
Comment on attachment 334397 [details] Patch for landing Clearing flags on attachment: 334397 Committed r228892: <https://trac.webkit.org/changeset/228892>