WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
124053
JSManagedValue should automatically call removeManagedReference:withOwner: upon dealloc
https://bugs.webkit.org/show_bug.cgi?id=124053
Summary
JSManagedValue should automatically call removeManagedReference:withOwner: up...
Mark Hahnenberg
Reported
2013-11-08 10:47:02 PST
This would make managing JSManagedValue lifetime much easier.
Attachments
Patch
(12.63 KB, patch)
2013-11-08 11:21 PST
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(13.14 KB, patch)
2014-02-06 11:21 PST
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(13.22 KB, patch)
2014-02-06 11:35 PST
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(13.54 KB, patch)
2014-02-06 12:29 PST
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(13.54 KB, patch)
2014-02-06 13:21 PST
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(13.68 KB, patch)
2014-02-06 14:19 PST
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(14.21 KB, patch)
2014-02-06 14:26 PST
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Mark Hahnenberg
Comment 1
2013-11-08 11:21:57 PST
Created
attachment 216414
[details]
Patch
Mark Hahnenberg
Comment 2
2013-11-08 11:22:32 PST
(In reply to
comment #1
)
> Created an attachment (id=216414) [details] > Patch
Still needs API approval and availability macros.
Mark Hahnenberg
Comment 3
2013-11-08 11:23:08 PST
<
rdar://problem/15409543
>
Oliver Hunt
Comment 4
2013-11-08 11:30:05 PST
Comment on
attachment 216414
[details]
Patch What will this do to clients that already do the work to avoid the leak? Are they all using the existing api?
Geoffrey Garen
Comment 5
2013-11-08 11:31:29 PST
Comment on
attachment 216414
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=216414&action=review
r=me, pending those things.
> Source/JavaScriptCore/API/JSManagedValue.h:58 > ++ (JSManagedValue *)managedValueWithValue:(JSValue *)value andOwner:(id)owner;
It looks like the canonical ObjC API idiom is to leave off "and". For example, -[NSString – initWithBytes:length:encoding:].
Mark Hahnenberg
Comment 6
2013-11-08 11:32:49 PST
(In reply to
comment #4
)
> (From update of
attachment 216414
[details]
) > What will this do to clients that already do the work to avoid the leak? Are they all using the existing api?
It will work correctly. Calling -removeManagedReference:withOwner: will remove the owner from the JSManagedValue's m_owners so when it is dealloc-ed it won't be called again.
Mark Rowe (bdash)
Comment 7
2013-11-13 14:46:31 PST
Can you post a new patch with availability macros added?
Mark Hahnenberg
Comment 8
2013-11-13 16:58:45 PST
(In reply to
comment #7
)
> Can you post a new patch with availability macros added?
I'm still not sure what macros to use. NS_AVAILABLE and friends will break the open source bots. Should I add a new WebKit availability macro for the current WebKit version using the new versioning scheme (e.g. 0x2100500 == 528.5.0) in JavaScriptCore/API/WebKitAvailability.h?
Mark Hahnenberg
Comment 9
2014-02-06 11:21:55 PST
Created
attachment 223353
[details]
Patch
WebKit Commit Bot
Comment 10
2014-02-06 11:23:14 PST
Attachment 223353
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:37: Missing spaces around = [whitespace/operators] [4] ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:41: Missing spaces around = [whitespace/operators] [4] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 11
2014-02-06 11:35:54 PST
Created
attachment 223356
[details]
Patch
WebKit Commit Bot
Comment 12
2014-02-06 11:37:56 PST
Attachment 223356
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:38: Missing spaces around = [whitespace/operators] [4] ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:42: Missing spaces around = [whitespace/operators] [4] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 13
2014-02-06 12:29:10 PST
Created
attachment 223368
[details]
Patch
WebKit Commit Bot
Comment 14
2014-02-06 12:31:51 PST
Attachment 223368
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:49: Missing spaces around = [whitespace/operators] [4] ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:53: Missing spaces around = [whitespace/operators] [4] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 15
2014-02-06 13:21:11 PST
Created
attachment 223373
[details]
Patch
WebKit Commit Bot
Comment 16
2014-02-06 13:22:26 PST
Attachment 223373
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:49: Missing spaces around = [whitespace/operators] [4] ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:53: Missing spaces around = [whitespace/operators] [4] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 17
2014-02-06 14:19:03 PST
Created
attachment 223382
[details]
Patch
WebKit Commit Bot
Comment 18
2014-02-06 14:21:28 PST
Attachment 223382
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:49: Missing spaces around = [whitespace/operators] [4] ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:53: Missing spaces around = [whitespace/operators] [4] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 19
2014-02-06 14:26:05 PST
Created
attachment 223384
[details]
Patch
WebKit Commit Bot
Comment 20
2014-02-06 14:28:53 PST
Attachment 223384
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:49: Missing spaces around = [whitespace/operators] [4] ERROR: Source/JavaScriptCore/API/WebKitAvailability.h:53: Missing spaces around = [whitespace/operators] [4] ERROR: Source/JavaScriptCore/API/JSManagedValue.h:74: Missing space after , [whitespace/comma] [3] ERROR: Source/JavaScriptCore/API/JSManagedValue.h:76: Missing space after , [whitespace/comma] [3] Total errors found: 4 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Geoffrey Garen
Comment 21
2014-02-06 15:50:23 PST
Comment on
attachment 223384
[details]
Patch r=me
WebKit Commit Bot
Comment 22
2014-02-06 16:25:04 PST
Comment on
attachment 223384
[details]
Patch Clearing flags on attachment: 223384 Committed
r163574
: <
http://trac.webkit.org/changeset/163574
>
WebKit Commit Bot
Comment 23
2014-02-06 16:25:07 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug