Bug 155681 - Purge PassRefPtr from WebCore/html/shadow
Summary: Purge PassRefPtr from WebCore/html/shadow
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joonghun Park
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-19 07:27 PDT by Joonghun Park
Modified: 2016-03-19 18:57 PDT (History)
1 user (show)

See Also:

Attachments
Patch (17.19 KB, patch)
2016-03-19 08:06 PDT, Joonghun Park 		no flags Details | Formatted Diff | Diff
Patch for landing (22.06 KB, patch)
2016-03-19 17:05 PDT, Joonghun Park 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joonghun Park 2016-03-19 07:27:58 PDT 
Purge PassRefPtr from WebCore/html/shadow.
Comment 1 Joonghun Park 2016-03-19 08:06:34 PDT 
Created attachment 274514 [details]
Patch
Comment 2 Darin Adler 2016-03-19 09:41:02 PDT 
Comment on attachment 274514 [details]
Patch

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

I prefer the name tryCreate to maybeCreate. The name "maybe create" makes it sound like the decision of whether to create or not is a policy choice. But it’s not, it’s an error handling strategy, and "try create" expresses that more clearly.

> Source/WebCore/html/shadow/MediaControlsApple.cpp:511
> +    Ref<EventListener> listener = eventListener();

I see no reason for this local variable to be a Ref<EventListener> instead of EventListener&.
Comment 3 Joonghun Park 2016-03-19 17:05:29 PDT 
Created attachment 274527 [details]
Patch for landing
Comment 4 Joonghun Park 2016-03-19 17:11:14 PDT 
Comment on attachment 274514 [details]
Patch

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

I changed maybeCreateFoo to tryCreateFoo where it just handles null error case.

>> Source/WebCore/html/shadow/MediaControlsApple.cpp:511
>> +    Ref<EventListener> listener = eventListener();
> 
> I see no reason for this local variable to be a Ref<EventListener> instead of EventListener&.

I changed this to EventListener& instead.
Comment 5 WebKit Commit Bot 2016-03-19 18:57:38 PDT 
Comment on attachment 274527 [details]
Patch for landing

Clearing flags on attachment: 274527

Committed r198473: <http://trac.webkit.org/changeset/198473>
Comment 6 WebKit Commit Bot 2016-03-19 18:57:41 PDT 
All reviewed patches have been landed.  Closing bug.