Bug 138174 – Eliminate ResourceBuffer and use SharedBuffer directly instead

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Darin Adler]

Eliminate ResourceBuffer and use SharedBuffer directly instead

[Darin Adler]

Created attachment 240607 [details]
Patch

[Darin Adler]

Looks like I didn’t update the EFL build system to remove the reference to ResourceBuffer.

[Darin Adler]

Still interested in review of this first cut, though.

[Antti Koivisto]

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.

[Darin Adler]

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.

[Darin Adler]

Created attachment 240673 [details]
Patch

[Darin Adler]

Created attachment 240675 [details]
Patch

[Darin Adler]

Created attachment 240679 [details]
Patch

[Darin Adler]

Created attachment 240721 [details]
Patch

[Darin Adler]

Committed r175406: <http://trac.webkit.org/changeset/175406>

[Csaba Osztrogonác]

(In reply to comment #10)
> Committed r175406: <http://trac.webkit.org/changeset/175406>

It made 15 tests crash on Apple's debug bots.

[Antti Koivisto]

Looking

[Antti Koivisto]

http://trac.webkit.org/changeset/175423 should help

[Alexey Proskuryakov]

Some tests are still broken after this change.

https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK1%20(Tests)/r175469%20(258)/results.html

[Alexey Proskuryakov]

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)

[WebKit Commit Bot]

Re-opened since this is blocked by bug 138327

[Alexey Proskuryakov]

Rolled out the patch and follow-up fixes in <https://trac.webkit.org/r175491>.

[Darin Adler]

Committed r175549: <http://trac.webkit.org/changeset/175549>

[Darin Adler]

Oops, one test still failing. Opening a new bug to fix that.

[Andy Estes]

Landed an iOS build fix: http://trac.webkit.org/changeset/175552