Bug 125231 - [EFL][GTK][WK2] Remove unnecessary reinterpret_casts when setting API clients
Summary: [EFL][GTK][WK2] Remove unnecessary reinterpret_casts when setting API clients
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nick Diego Yamane (diegoyam)
URL:
Keywords:
Depends on: 125240
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-04 10:54 PST by Nick Diego Yamane (diegoyam)
Modified: 2013-12-04 14:40 PST (History)
10 users (show)

See Also:


Attachments
Patch (12.80 KB, patch)
2013-12-04 11:38 PST, Nick Diego Yamane (diegoyam)
no flags Details | Formatted Diff | Diff
Rebased patch (14.07 KB, patch)
2013-12-04 11:47 PST, Nick Diego Yamane (diegoyam)
no flags Details | Formatted Diff | Diff
Patch (12.05 KB, patch)
2013-12-04 14:21 PST, Nick Diego Yamane (diegoyam)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Diego Yamane (diegoyam) 2013-12-04 10:54:49 PST
Bug 125182 changed modified GTK and EFL API Clients to follow the new API::Client implementation.
It introduced some unnecessary reinterpret_casts when setting those clients. This bug is for removing them.
Comment 1 Nick Diego Yamane (diegoyam) 2013-12-04 11:38:03 PST
Created attachment 218425 [details]
Patch
Comment 2 Nick Diego Yamane (diegoyam) 2013-12-04 11:47:37 PST
Created attachment 218426 [details]
Rebased patch
Comment 3 WebKit Commit Bot 2013-12-04 11:49:24 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 Martin Robinson 2013-12-04 11:50:40 PST
Comment on attachment 218426 [details]
Rebased patch

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

Looks good apart from the ChangeLog weirdness.

> Source/WebKit2/ChangeLog:72
> +        * UIProcess/API/gtk/WebKitFullscreenClient.cpp:
> +        (attachFullScreenClientToView):
> +        * UIProcess/API/gtk/WebKitRequestManagerClient.cpp:
> +        (attachRequestManagerClientToContext):
> +        * UIProcess/API/gtk/WebKitTextChecker.cpp:
> +        (WebKitTextChecker::WebKitTextChecker):
> +        * UIProcess/API/gtk/WebKitWebInspector.cpp:
> +        (webkitWebInspectorCreate):
> +        * UIProcess/efl/BatteryProvider.cpp:
> +        (BatteryProvider::BatteryProvider):
> +        * UIProcess/efl/NetworkInfoProvider.cpp:
> +        (NetworkInfoProvider::NetworkInfoProvider):
> +        * UIProcess/efl/RequestManagerClientEfl.cpp:
> +        (WebKit::RequestManagerClientEfl::RequestManagerClientEfl):
> +        * UIProcess/efl/TextCheckerClientEfl.cpp:
> +        (TextCheckerClientEfl::TextCheckerClientEfl):
> +        * UIProcess/efl/VibrationClientEfl.cpp:
> +        (VibrationClientEfl::VibrationClientEfl):
> +        * UIProcess/efl/ViewClientEfl.cpp:

It seems wrong to update an old ChangeLog entry.
Comment 5 Nick Diego Yamane (diegoyam) 2013-12-04 12:08:28 PST
(In reply to comment #4)
> (From update of attachment 218426 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218426&action=review
> 
> Looks good apart from the ChangeLog weirdness.
> 
> > Source/WebKit2/ChangeLog:72
> > +        * UIProcess/API/gtk/WebKitFullscreenClient.cpp:
> > +        (attachFullScreenClientToView):
> > +        * UIProcess/API/gtk/WebKitRequestManagerClient.cpp:
> > +        (attachRequestManagerClientToContext):
> > +        * UIProcess/API/gtk/WebKitTextChecker.cpp:
> > +        (WebKitTextChecker::WebKitTextChecker):
> > +        * UIProcess/API/gtk/WebKitWebInspector.cpp:
> > +        (webkitWebInspectorCreate):
> > +        * UIProcess/efl/BatteryProvider.cpp:
> > +        (BatteryProvider::BatteryProvider):
> > +        * UIProcess/efl/NetworkInfoProvider.cpp:
> > +        (NetworkInfoProvider::NetworkInfoProvider):
> > +        * UIProcess/efl/RequestManagerClientEfl.cpp:
> > +        (WebKit::RequestManagerClientEfl::RequestManagerClientEfl):
> > +        * UIProcess/efl/TextCheckerClientEfl.cpp:
> > +        (TextCheckerClientEfl::TextCheckerClientEfl):
> > +        * UIProcess/efl/VibrationClientEfl.cpp:
> > +        (VibrationClientEfl::VibrationClientEfl):
> > +        * UIProcess/efl/ViewClientEfl.cpp:
> 
> It seems wrong to update an old ChangeLog entry.

Oops, maybe the git rebase has done this :(

I'll fix that.

But it looks like there are other build issues, probably result from r160104.
I'll try to fix that firstly.
Comment 6 Nick Diego Yamane (diegoyam) 2013-12-04 14:21:23 PST
Created attachment 218447 [details]
Patch
Comment 7 WebKit Commit Bot 2013-12-04 14:39:59 PST
Comment on attachment 218447 [details]
Patch

Clearing flags on attachment: 218447

Committed r160128: <http://trac.webkit.org/changeset/160128>
Comment 8 WebKit Commit Bot 2013-12-04 14:40:02 PST
All reviewed patches have been landed.  Closing bug.