Bug 140479 – WeakPtr functions crash when created with default constructor

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

WeakPtr functions crash when created with default constructor

[Myles C. Maxfield]

Created attachment 244662 [details]
Patch

[WebKit Commit Bot]

Attachment 244662 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/WeakPtr.h:92:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:101:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 244672 [details]
Patch

[WebKit Commit Bot]

Attachment 244672 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/WeakPtr.h:92:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:101:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

EFL is a false negative - it looks like the compiler was killed.

[Myles C. Maxfield]

Style is also a false negative - the style checker doesn't understand single line functions.

[Andreas Kling]

Comment on attachment 244672 [details]
Patch

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

> Source/WTF/wtf/WeakPtr.h:101
> +    WeakPtr(Ref<WeakReference<T>> ref) : m_ref(ref) { }

Shouldn't this be:
WeakPtr(Ref<WeakReference<T>>&& ref) : m_ref(WTF::move(ref)) { }

[Myles C. Maxfield]

Comment on attachment 244672 [details]
Patch

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

>> Source/WTF/wtf/WeakPtr.h:101
>> +    WeakPtr(Ref<WeakReference<T>> ref) : m_ref(ref) { }
> 
> Shouldn't this be:
> WeakPtr(Ref<WeakReference<T>>&& ref) : m_ref(WTF::move(ref)) { }

I don't think so. I'm trying to copy the Ref that the caller uses when calling this function.

[Anders Carlsson]

(In reply to comment #8)
> Comment on attachment 244672 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244672&action=review
> 
> >> Source/WTF/wtf/WeakPtr.h:101
> >> +    WeakPtr(Ref<WeakReference<T>> ref) : m_ref(ref) { }
> > 
> > Shouldn't this be:
> > WeakPtr(Ref<WeakReference<T>>&& ref) : m_ref(WTF::move(ref)) { }
> 
> I don't think so. I'm trying to copy the Ref that the caller uses when
> calling this function.

Then you should use copyRef() at the call site.

[Myles C. Maxfield]

Created attachment 244768 [details]
Patch

[WebKit Commit Bot]

Attachment 244768 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/WeakPtr.h:92:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:93:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:102:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 244779 [details]
Patch

[WebKit Commit Bot]

Attachment 244779 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/WeakPtr.h:92:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:93:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:106:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 244786 [details]
Patch

[WebKit Commit Bot]

Attachment 244786 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/WeakPtr.h:92:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:93:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/WeakPtr.h:106:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andreas Kling]

Comment on attachment 244786 [details]
Patch

r=me

We really should make more use of this class.

[WebKit Commit Bot]

Comment on attachment 244786 [details]
Patch

Clearing flags on attachment: 244786

Committed r178615: <http://trac.webkit.org/changeset/178615>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

(In reply to comment #17)
> Comment on attachment 244786 [details]
> Patch
> 
> Clearing flags on attachment: 244786
> 
> Committed r178615: <http://trac.webkit.org/changeset/178615>

Build fix for GTK once patch in Bug 155394 lands (since WeakPtr.cpp was not added to Tools/TestWebKitAPI/CMakeLists.txt when this patch originally landed):

Committed r198067.  <http://trac.webkit.org/changeset/198067>