| Summary: | Eliminate ResourceBuffer and use SharedBuffer directly instead | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||
| Component: | Page Loading | Assignee: | Darin Adler <darin> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | aestes, ap, beidson, commit-queue, kling, koivisto, ossy | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | 138327 | ||||||||||||||
| Bug Blocks: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Darin Adler
2014-10-29 08:28:52 PDT
Created attachment 240607 [details]
Patch
Looks like I didn’t update the EFL build system to remove the reference to ResourceBuffer. Still interested in review of this first cut, though. Comment on attachment 240607 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240607&action=review > Source/WebCore/loader/cache/CachedResource.h:361 > +class CachedResource::Callback { > +public: > + Callback(CachedResource&, CachedResourceClient&); > + > + void cancel(); > + > +private: > + void timerFired(Timer<Callback>&); > + > + CachedResource& m_resource; > + CachedResourceClient& m_client; > + Timer<Callback> m_timer; > +}; You could probably remove this awkward type and use a lambda. Comment on attachment 240607 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240607&action=review >> Source/WebCore/loader/cache/CachedResource.h:361 >> +}; > > You could probably remove this awkward type and use a lambda. Good direction; but for the moment, not sure how to use a lambda with a Timer since there’s no std::function version yet. Created attachment 240673 [details]
Patch
Created attachment 240675 [details]
Patch
Created attachment 240679 [details]
Patch
Created attachment 240721 [details]
Patch
Committed r175406: <http://trac.webkit.org/changeset/175406> (In reply to comment #10) > Committed r175406: <http://trac.webkit.org/changeset/175406> It made 15 tests crash on Apple's debug bots. Looking http://trac.webkit.org/changeset/175423 should help Some tests are still broken after this change. https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK1%20(Tests)/r175469%20(258)/results.html CRASHING TEST:/multipart/stop-crash.html Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010d817bfa WTFCrash + 42 1 com.apple.WebCore 0x000000010f0ce16e WTF::PassRef<WebCore::SharedBuffer>::~PassRef() + 62 (PassRef.h:102) 2 com.apple.WebCore 0x000000010f0cda15 WTF::PassRef<WebCore::SharedBuffer>::~PassRef() + 21 (PassRef.h:103) 3 com.apple.WebCore 0x0000000110aa440c WebCore::SubresourceLoader::didReceiveResponse(WebCore::ResourceResponse const&) + 764 (SubresourceLoader.cpp:239) 4 com.apple.WebKit 0x000000010ac27a87 WebKit::WebResourceLoader::didReceiveResponse(WebCore::ResourceResponse const&, bool) + 311 (WebResourceLoader.cpp:129) 5 com.apple.WebKit 0x000000010ac2b6fa void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool), std::__1::tuple<WebCore::ResourceResponse, bool>, 0ul, 1ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::ResourceResponse const&, bool), std::__1::tuple<WebCore::ResourceResponse, bool>&&, std::index_sequence<0ul, 1ul>) + 186 (HandleMessage.h:17) Re-opened since this is blocked by bug 138327 Rolled out the patch and follow-up fixes in <https://trac.webkit.org/r175491>. Committed r175549: <http://trac.webkit.org/changeset/175549> Oops, one test still failing. Opening a new bug to fix that. Landed an iOS build fix: http://trac.webkit.org/changeset/175552 |