Summary: | Make Functional bind PassOwnPtr. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dongseong Hwang <dongseong.hwang> | ||||||
Component: | Web Template Framework | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | UNCONFIRMED --- | ||||||||
Severity: | Normal | CC: | andersca, barraclough, darin, eric, sam, skyul | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 91310 | ||||||||
Attachments: |
|
Description
Dongseong Hwang
2012-07-13 05:07:22 PDT
Created attachment 152223 [details]
Patch
Comment on attachment 152223 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152223&action=review > Source/WTF/ChangeLog:8 > + In common, threads communicate using Functional. However, Functional causes the Functional is commonly used for inter-thread communication. > Tools/TestWebKitAPI/Tests/WTF/Functional.cpp:221 > +class OwnershipClass { This class is a own-ptr version of Number class above. Why don't we name it OwnedNumber or something instead of OwnershipClass? Comment on attachment 152223 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152223&action=review > Source/WTF/wtf/Functional.h:322 > + static void deref(const OwnPtr<T>&) { } I am curious why we need const here. (In reply to comment #2) > (From update of attachment 152223 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152223&action=review > > > Source/WTF/ChangeLog:8 > > + In common, threads communicate using Functional. However, Functional causes the > > Functional is commonly used for inter-thread communication. > > > Tools/TestWebKitAPI/Tests/WTF/Functional.cpp:221 > > +class OwnershipClass { > > This class is a own-ptr version of Number class above. Why don't we name it OwnedNumber or something instead of OwnershipClass? Good point. I will update. (In reply to comment #3) > (From update of attachment 152223 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152223&action=review > > > Source/WTF/wtf/Functional.h:322 > > + static void deref(const OwnPtr<T>&) { } > > I am curious why we need const here. "const" is not necessary. Putting "const" in front of a reference is my habit. However, I think it is not bad. Created attachment 152374 [details]
Patch
(In reply to comment #6) > Created an attachment (id=152374) [details] > Patch Amended Changelog to correct the grammar. Renamed the class name for test from OwnershipClass to OwnedNumber. I'm waiting the review. I filed this bug because I wanted to use the Functional with OwnPtr in parallel image decoders (Bug 90375). Currently, I used foo(Bar*) with 'delete bar' instead of foo(PassOwnPtr<Bar>). Comment on attachment 152374 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152374&action=review > Source/WTF/wtf/Functional.h:322 > +template<typename T, bool shouldRefAndDeref> struct RefAndDeref<PassOwnPtr<T>, shouldRefAndDeref> { > + static void ref(const OwnPtr<T>&) { } > + static void deref(const OwnPtr<T>&) { } > +}; Is this really needed? Comment on attachment 152374 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152374&action=review >> Source/WTF/wtf/Functional.h:322 >> +}; > > Is this really needed? Yes, it is needed because the StorageType of PassOwnPtr is OwnPtr. In "RefAndDeref<P1, FunctionWrapper::shouldRefFirstParameter>::ref(m_p1)", P1 is PassOwnPtr but m_p1 is OwnPtr, which is the StorageType of PassOwnPtr. So if this code stub is removed, we will encounter a compile error like the following in gcc. " ../../../Source/WTF/wtf/Functional.h: In constructor ‘WTF::BoundFunctionImpl<FunctionWrapper, R(P1)>::BoundFunctionImpl(FunctionWrapper, const P1&) [with FunctionWrapper = WTF::FunctionWrapper<int (*)(WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber>)>, R = int, P1 = WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber>]’: ../../../Source/WTF/wtf/Functional.h:662:240: instantiated from ‘WTF::Function<typename WTF::FunctionWrapper<FunctionType>::ResultType()> WTF::bind(FunctionType, const A1&) [with FunctionType = int (*)(WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber>), A1 = WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber>, typename WTF::FunctionWrapper<FunctionType>::ResultType = int]’ ../../../Tools/TestWebKitAPI/Tests/WTF/Functional.cpp:263:72: instantiated from here ../../../Source/WTF/wtf/Functional.h:404:9: error: no matching function for call to ‘WTF::RefAndDeref<WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber>, false>::ref(WTF::ParamStorageTraits<WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber> >::StorageType&)’ ../../../Source/WTF/wtf/Functional.h:404:9: note: candidate is: ../../../Source/WTF/wtf/Functional.h:315:17: note: static void WTF::RefAndDeref<T, shouldRefAndDeref>::ref(T) [with T = WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber>, bool shouldRefAndDeref = false] ../../../Source/WTF/wtf/Functional.h:315:17: note: no known conversion for argument 1 from ‘WTF::ParamStorageTraits<WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber> >::StorageType {aka WTF::OwnPtr<TestWebKitAPI::OwnedNumber>}’ to ‘WTF::PassOwnPtr<TestWebKitAPI::OwnedNumber>’ " Comment on attachment 152374 [details]
Patch
Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.
|