Bug 140479 - WeakPtr functions crash when created with default constructor
Summary: WeakPtr functions crash when created with default constructor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-14 16:28 PST by Myles C. Maxfield
Modified: 2016-03-12 13:01 PST (History)
12 users (show)

See Also:


Attachments
Patch (6.74 KB, patch)
2015-01-14 16:46 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (16.36 KB, patch)
2015-01-14 18:19 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (16.69 KB, patch)
2015-01-16 09:49 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (15.66 KB, patch)
2015-01-16 11:29 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (16.38 KB, patch)
2015-01-16 12:19 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-01-14 16:28:07 PST
WeakPtr functions crash when created with default constructor
Comment 1 Myles C. Maxfield 2015-01-14 16:46:25 PST
Created attachment 244662 [details]
Patch
Comment 2 WebKit Commit Bot 2015-01-14 16:48:24 PST
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.
Comment 3 Myles C. Maxfield 2015-01-14 18:19:26 PST
Created attachment 244672 [details]
Patch
Comment 4 WebKit Commit Bot 2015-01-14 18:21:22 PST
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.
Comment 5 Myles C. Maxfield 2015-01-14 20:59:33 PST
EFL is a false negative - it looks like the compiler was killed.
Comment 6 Myles C. Maxfield 2015-01-15 07:28:17 PST
Style is also a false negative - the style checker doesn't understand single line functions.
Comment 7 Andreas Kling 2015-01-15 11:22:22 PST
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)) { }
Comment 8 Myles C. Maxfield 2015-01-15 15:48:06 PST
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.
Comment 9 Anders Carlsson 2015-01-15 15:54:23 PST
(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.
Comment 10 Myles C. Maxfield 2015-01-16 09:49:04 PST
Created attachment 244768 [details]
Patch
Comment 11 WebKit Commit Bot 2015-01-16 09:50:53 PST
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.
Comment 12 Myles C. Maxfield 2015-01-16 11:29:35 PST
Created attachment 244779 [details]
Patch
Comment 13 WebKit Commit Bot 2015-01-16 11:33:26 PST
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.
Comment 14 Myles C. Maxfield 2015-01-16 12:19:01 PST
Created attachment 244786 [details]
Patch
Comment 15 WebKit Commit Bot 2015-01-16 12:21:46 PST
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.
Comment 16 Andreas Kling 2015-01-16 15:31:21 PST
Comment on attachment 244786 [details]
Patch

r=me

We really should make more use of this class.
Comment 17 WebKit Commit Bot 2015-01-16 16:39:56 PST
Comment on attachment 244786 [details]
Patch

Clearing flags on attachment: 244786

Committed r178615: <http://trac.webkit.org/changeset/178615>
Comment 18 WebKit Commit Bot 2015-01-16 16:40:00 PST
All reviewed patches have been landed.  Closing bug.
Comment 19 David Kilzer (:ddkilzer) 2016-03-12 13:01:33 PST
(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>