Bug 73540 – [EFL] Cleanup includes to reduce code complexity.

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Ryuan Choi]

EWebKit.h is a list of public header files for application to use WebKit/Efl.
For now, internal source files use it.

I'd like to cleanup includes in order to reduce code complexity,

[Ryuan Choi]

Created attachment 117368 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 117368 [details]
Patch

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

The patch looks OK, but I'd like to have the issue below clarified before giving my informal r+.

> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:-41
> -#if ENABLE(SQL_DATABASE)
> -
> -#include "DatabaseDetails.h"
> -#include "DatabaseTracker.h"
> -#endif

I've always wondered what the correct place to put #includes inside #if clauses was. The style guide does not have an item for this, so can you clarify why these should be moved down?

[Gustavo Noronha (kov)]

Comment on attachment 117368 [details]
Patch

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

>> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:-41
>> -#endif
> 
> I've always wondered what the correct place to put #includes inside #if clauses was. The style guide does not have an item for this, so can you clarify why these should be moved down?

I don't remember where I read this, but I do remember reading somewhere that having the headers included conditionally should be at the bottom, indeed.

> Source/WebKit/efl/ewk/ewk_frame.cpp:55
>  #include "ewk_private.h"
>  
>  #include <Eina.h>

This empty line should not exist.

> Source/WebKit/efl/ewk/ewk_private.h:39
> +#include "ewk_view.h"
>  
> -#include <cairo.h>
> +#include <Evas.h>

Same here.

> Source/WebKit/efl/ewk/ewk_window_features.cpp:28
>  #include "WindowFeatures.h"
> +#include "ewk_logging.h"
>  #include "ewk_private.h"
>  
>  #include <Eina.h>

Ditto.

[Ryuan Choi]

Created attachment 117506 [details]
Patch

[Ryuan Choi]

(In reply to comment #3)
> (From update of attachment 117368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117368&action=review
> 
> >> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:-41
> >> -#endif
> > 
> > I've always wondered what the correct place to put #includes inside #if clauses was. The style guide does not have an item for this, so can you clarify why these should be moved down?
> 
> I don't remember where I read this, but I do remember reading somewhere that having the headers included conditionally should be at the bottom, indeed.
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:55
> >  #include "ewk_private.h"
> >  
> >  #include <Eina.h>
> 
> This empty line should not exist.
> 
> > Source/WebKit/efl/ewk/ewk_private.h:39
> > +#include "ewk_view.h"
> >  
> > -#include <cairo.h>
> > +#include <Evas.h>
> 
> Same here.
> 
> > Source/WebKit/efl/ewk/ewk_window_features.cpp:28
> >  #include "WindowFeatures.h"
> > +#include "ewk_logging.h"
> >  #include "ewk_private.h"
> >  
> >  #include <Eina.h>
> 
> Ditto.

Thanks for your review.

I fixed you mentioned.

[WebKit Review Bot]

Comment on attachment 117506 [details]
Patch

Rejecting attachment 117506 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
wk/ewk_settings.cpp
patching file Source/WebKit/efl/ewk/ewk_util.cpp
patching file Source/WebKit/efl/ewk/ewk_view.cpp
Hunk #2 FAILED at 48.
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_view.cpp.rej
patching file Source/WebKit/efl/ewk/ewk_view_single.cpp
patching file Source/WebKit/efl/ewk/ewk_view_tiled.cpp
patching file Source/WebKit/efl/ewk/ewk_window_features.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/10725142

[Ryuan Choi]

Created attachment 117547 [details]
Patch

[Ryuan Choi]

Comment on attachment 117547 [details]
Patch

rebased.

[WebKit Review Bot]

Comment on attachment 117547 [details]
Patch

Clearing flags on attachment: 117547

Committed r101744: <http://trac.webkit.org/changeset/101744>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.