Bug 88803

Summary: [EFL] Move cursor related code from WidgetEfl to ewk_view and EflScreenUtilities.
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: cshu, gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 88633    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

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.