Bug 111706 - [WK2][EFL] Fix EwkContext class API
Summary: [WK2][EFL] Fix EwkContext class API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords:
Depends on:
Blocks: 111543
  Show dependency treegraph
 
Reported: 2013-03-07 03:30 PST by Mikhail Pozdnyakov
Modified: 2013-03-12 05:34 PDT (History)
8 users (show)

See Also:


Attachments
patch (5.53 KB, patch)
2013-03-07 04:02 PST, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff
patch v2 (5.60 KB, patch)
2013-03-07 05:07 PST, Mikhail Pozdnyakov
ap: review+
Details | Formatted Diff | Diff
to be landed (5.52 KB, patch)
2013-03-12 05:05 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 2013-03-07 03:30:29 PST
EwkContext class has couple of problems to be fixed:
1) EwkContext::create() should be called getOrCreate actually, as it returns the same instance of EwkContext for the given WKContextRef if present.
2) EwkContext::defaultContext() should return raw pointer rather than PassRefPtr<EwkContext> as ownership is not transferred there.
Comment 1 Mikhail Pozdnyakov 2013-03-07 04:02:57 PST
Created attachment 191964 [details]
patch
Comment 2 Kenneth Rohde Christiansen 2013-03-07 04:04:37 PST
Comment on attachment 191964 [details]
patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:105
> +PassRefPtr<EwkContext> EwkContext::getOrCreate(WKContextRef context)

I really dislike the name! Ain't there examples of similar things in WebCore
Comment 3 Mikhail Pozdnyakov 2013-03-07 05:07:11 PST
Created attachment 191974 [details]
patch v2

renamed getOrCreate(). Default context is created after main() entrance :)
Comment 4 Kenneth Rohde Christiansen 2013-03-08 03:32:27 PST
Comment on attachment 191974 [details]
patch v2

LGTM
Comment 5 Alexey Proskuryakov 2013-03-11 10:59:15 PDT
Comment on attachment 191974 [details]
patch v2

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:132
> +    static EwkContext* defaultInstance = 0;
> +    if (!defaultInstance)
> +        defaultInstance = create().leakRef();

A better way to write this would be:

static EwkContext* defaultInstance = create().leakRef();
Comment 6 Mikhail Pozdnyakov 2013-03-12 05:05:18 PDT
Created attachment 192713 [details]
to be landed
Comment 7 WebKit Review Bot 2013-03-12 05:34:50 PDT
Comment on attachment 192713 [details]
to be landed

Clearing flags on attachment: 192713

Committed r145535: <http://trac.webkit.org/changeset/145535>
Comment 8 WebKit Review Bot 2013-03-12 05:34:55 PDT
All reviewed patches have been landed.  Closing bug.