Bug 88803 - [EFL] Move cursor related code from WidgetEfl to ewk_view and EflScreenUtilities.
Summary: [EFL] Move cursor related code from WidgetEfl to ewk_view and EflScreenUtilit...
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: Ryuan Choi
URL:
Keywords:
Depends on: 88633
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-11 13:51 PDT by Ryuan Choi
Modified: 2012-06-14 16:10 PDT (History)
6 users (show)

See Also:


Attachments
Patch (15.54 KB, patch)
2012-06-11 14:37 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (15.90 KB, patch)
2012-06-11 14:56 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (15.94 KB, patch)
2012-06-11 16:28 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (15.90 KB, patch)
2012-06-12 21:31 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (15.93 KB, patch)
2012-06-13 03:25 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2012-06-11 13:51:38 PDT
Because WebProcess can not control cursor,
setCursor should be passed to UIProcess via ChromeClient.

As a second step,
Move and refactor cursor related code from WidgetEfl to ewk_view and EflScreenUtilities.
Comment 1 Ryuan Choi 2012-06-11 14:37:07 PDT
Created attachment 146916 [details]
Patch
Comment 2 Ryuan Choi 2012-06-11 14:56:30 PDT
Created attachment 146921 [details]
Patch
Comment 3 Ryuan Choi 2012-06-11 16:28:55 PDT
Created attachment 146957 [details]
Patch
Comment 4 Gyuyoung Kim 2012-06-12 18:50:03 PDT
Comment on attachment 146957 [details]
Patch

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

Are there test cases covered by this patch ?

> Source/WebCore/platform/efl/EflScreenUtilities.cpp:115
> +    Ecore_X_Window win = ecore_evas_software_x11_window_get(ecoreEvas);

I prefer to use full name for variable. s/win/window/g, s/cur/cursor/g
Comment 5 Ryuan Choi 2012-06-12 21:31:55 PDT
Created attachment 147223 [details]
Patch
Comment 6 Ryuan Choi 2012-06-12 21:33:24 PDT
(In reply to comment #4)
> (From update of attachment 146957 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146957&action=review
> 
> Are there test cases covered by this patch ?
> 
I am not sure whether we can test cursor.

At least DRT/Efl can not test it because we just calls WebCore::paint.

> > Source/WebCore/platform/efl/EflScreenUtilities.cpp:115
> > +    Ecore_X_Window win = ecore_evas_software_x11_window_get(ecoreEvas);
> 
> I prefer to use full name for variable. s/win/window/g, s/cur/cursor/g

OK, I fixed.
Comment 7 Gyuyoung Kim 2012-06-13 03:08:52 PDT
Comment on attachment 147223 [details]
Patch

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

From the viewpoint of refactoring, looks fine except for trivial nit.

> Source/WebCore/platform/efl/EflScreenUtilities.cpp:134
> +    const char *engine = ecore_evas_engine_name_get(ecoreEvas);

Style nit : Move '*' to variable side.
Comment 8 Ryuan Choi 2012-06-13 03:25:09 PDT
Created attachment 147275 [details]
Patch
Comment 9 Ryuan Choi 2012-06-13 03:26:03 PDT
(In reply to comment #7)
> (From update of attachment 147223 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147223&action=review
> 
> From the viewpoint of refactoring, looks fine except for trivial nit.
> 
> > Source/WebCore/platform/efl/EflScreenUtilities.cpp:134
> > +    const char *engine = ecore_evas_engine_name_get(ecoreEvas);
> 
> Style nit : Move '*' to variable side.

Thanks, fixed.
Comment 10 Gyuyoung Kim 2012-06-13 19:22:24 PDT
LGTM.
Comment 11 Chang Shu 2012-06-14 07:16:45 PDT
Comment on attachment 147275 [details]
Patch

r+ based on Gyuyoung's comments.
Comment 12 Ryuan Choi 2012-06-14 15:32:11 PDT
Comment on attachment 147275 [details]
Patch

thank you.
Comment 13 WebKit Review Bot 2012-06-14 16:10:02 PDT
Comment on attachment 147275 [details]
Patch

Clearing flags on attachment: 147275

Committed r120369: <http://trac.webkit.org/changeset/120369>
Comment 14 WebKit Review Bot 2012-06-14 16:10:08 PDT
All reviewed patches have been landed.  Closing bug.