Bug 144981 - Use Ref instead of PassRefPtr in WebCore/bindings
Summary: Use Ref instead of PassRefPtr in WebCore/bindings
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: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks: 144092
  Show dependency treegraph
 
Reported: 2015-05-13 18:36 PDT by Gyuyoung Kim
Modified: 2015-05-18 22:55 PDT (History)
3 users (show)

See Also:


Attachments
Patch (19.96 KB, patch)
2015-05-13 18:47 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch for landing (20.17 KB, patch)
2015-05-18 19:29 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch for landing (20.22 KB, patch)
2015-05-18 21:40 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2015-05-13 18:36:37 PDT
SSIA
Comment 1 Gyuyoung Kim 2015-05-13 18:47:26 PDT
Created attachment 253080 [details]
Patch
Comment 2 Darin Adler 2015-05-17 10:53:29 PDT
Comment on attachment 253080 [details]
Patch

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

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:2655
>                                                                  MessagePortArray* messagePorts, ArrayBufferArray* arrayBuffers,
>                                                                  SerializationErrorMode throwExceptions)

Not lined up any more. Better to put it all on one line,

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:2670
>          return 0;

nullptr?

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:2679
>          return 0;

nullptr?

> Source/WebCore/bindings/objc/ObjCEventListener.h:41
> +        static RefPtr<ObjCEventListener> wrap(ObjCListener);

Should return Ref, not RefPtr. If you look at the implementation you’ll see it never returns null.

> Source/WebCore/bindings/objc/ObjCEventListener.mm:55
>          return wrapper;

Need to use releaseNonNull here to convert the RefPtr into a Ref.
Comment 3 Gyuyoung Kim 2015-05-18 19:29:15 PDT
Created attachment 253359 [details]
Patch for landing
Comment 4 Gyuyoung Kim 2015-05-18 19:29:51 PDT
(In reply to comment #2)
> Comment on attachment 253080 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=253080&action=review
> 
> > Source/WebCore/bindings/js/SerializedScriptValue.cpp:2655
> >                                                                  MessagePortArray* messagePorts, ArrayBufferArray* arrayBuffers,
> >                                                                  SerializationErrorMode throwExceptions)
> 
> Not lined up any more. Better to put it all on one line,
> 
> > Source/WebCore/bindings/js/SerializedScriptValue.cpp:2670
> >          return 0;
> 
> nullptr?
> 
> > Source/WebCore/bindings/js/SerializedScriptValue.cpp:2679
> >          return 0;
> 
> nullptr?
> 
> > Source/WebCore/bindings/objc/ObjCEventListener.h:41
> > +        static RefPtr<ObjCEventListener> wrap(ObjCListener);
> 
> Should return Ref, not RefPtr. If you look at the implementation you’ll see
> it never returns null.
> 
> > Source/WebCore/bindings/objc/ObjCEventListener.mm:55
> >          return wrapper;
> 
> Need to use releaseNonNull here to convert the RefPtr into a Ref.

All done ! Thanks.
Comment 5 WebKit Commit Bot 2015-05-18 20:20:53 PDT
Comment on attachment 253359 [details]
Patch for landing

Rejecting attachment 253359 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 253359, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/6377800806170624
Comment 6 Gyuyoung Kim 2015-05-18 21:40:41 PDT
Created attachment 253364 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2015-05-18 22:55:30 PDT
Comment on attachment 253364 [details]
Patch for landing

Clearing flags on attachment: 253364

Committed r184543: <http://trac.webkit.org/changeset/184543>
Comment 8 WebKit Commit Bot 2015-05-18 22:55:34 PDT
All reviewed patches have been landed.  Closing bug.