Bug 91338

Summary: [EFL][WK2] Define destructors for Ewk structures
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, haraken, kenneth, ryuan.choi, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 61838    
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 2012-07-15 00:51:53 PDT
Now that our Ewk structures have constructors, it makes sense to define destructors as well.
Attachments
Patch (9.11 KB, patch)
2012-07-15 01:24 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (9.31 KB, patch)
2012-07-15 05:06 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2012-07-15 01:24:18 PDT
Created attachment 152456 [details] Patch
Kentaro Hara
Comment 2 2012-07-15 02:16:36 PDT
Comment on attachment 152456 [details] Patch Looks reasonable. Let me r+ it after informal reviews from EFL folks.
Gyuyoung Kim
Comment 3 2012-07-15 04:15:15 PDT
Comment on attachment 152456 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152456&action=review > Source/WebKit2/UIProcess/API/efl/ewk_navigation_policy_decision.cpp:62 > + { Don't you need to add ASSERT(!__ref); here as well ?
Chris Dumez
Comment 4 2012-07-15 04:36:43 PDT
Comment on attachment 152456 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152456&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_navigation_policy_decision.cpp:62 >> + { > > Don't you need to add ASSERT(!__ref); here as well ? No, _Ewk_Navigation_Policy_Decision is not ref counted. There is no __ref member in the struct.
Chris Dumez
Comment 5 2012-07-15 05:03:19 PDT
(In reply to comment #3) > (From update of attachment 152456 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152456&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_navigation_policy_decision.cpp:62 > > + { > > Don't you need to add ASSERT(!__ref); here as well ? I'll make Ewk_Navigation_Policy_Decision ref counted in Bug 91343 though so I'll add the ASSERT there.
Chris Dumez
Comment 6 2012-07-15 05:06:00 PDT
Created attachment 152460 [details] Patch Move the default policy decision code to the Ewk_Navigation_Policy_Decision destructor as well.
Raphael Kubo da Costa (:rakuco)
Comment 7 2012-07-15 17:15:22 PDT
Comment on attachment 152460 [details] Patch Looks OK to me.
Gyuyoung Kim
Comment 8 2012-07-15 17:23:39 PDT
Comment on attachment 152460 [details] Patch LGTM too.
Kentaro Hara
Comment 9 2012-07-15 23:13:06 PDT
Comment on attachment 152460 [details] Patch Thanks for reviews. Looks OK to me.
WebKit Review Bot
Comment 10 2012-07-15 23:25:28 PDT
Comment on attachment 152460 [details] Patch Clearing flags on attachment: 152460 Committed r122697: <http://trac.webkit.org/changeset/122697>
WebKit Review Bot
Comment 11 2012-07-15 23:25:34 PDT
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.