Bug 64064 – [EFL] Add dummy NotificationPresenterClientEfl

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Gyuyoung Kim]

This patch adds dummy files of NotificationPresenterClientEfl to WebCoreSupport for HTML 5 Notification. This patch is to start to support it. Though ENABLE_NOTIFICATIONS was added to OptionEfl.cmake, the ENABLE_NOTIFICATIONS is disabled on EFL port now.

[Gyuyoung Kim]

Created attachment 99938 [details]
Proposed Patch

[Raphael Kubo da Costa (:rakuco)]

> Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.cpp:28
> +namespace WebCore {

You should not need both `using namespace WebCore' and `namespace WebCore'.

[Gyuyoung Kim]

(In reply to comment #2)
> > Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.cpp:28
> > +namespace WebCore {
> 
> You should not need both `using namespace WebCore' and `namespace WebCore'.

Why do you think the both is not needed ?

I think the NotificationPresenterClientEfl will use WebCore classes. So, I add the 'using namespace WebCore'. And, the functions of NotificationPresenterClientEfl will be used by ewk port. So, I think it is better to be wrapped by WebCore namespace. Because, ewk files are already using WebCore classes via 'WebCore::'

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #3)
> (In reply to comment #2)
> > > Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.cpp:28
> > > +namespace WebCore {
> > 
> > You should not need both `using namespace WebCore' and `namespace WebCore'.
> 
> Why do you think the both is not needed ?
> 
> I think the NotificationPresenterClientEfl will use WebCore classes. So, I add the 'using namespace WebCore'.

If the class is already inside the WebCore namespace, you do not need to add `using namespace WebCore' as well.

> And, the functions of NotificationPresenterClientEfl will be used by ewk port. So, I think it is better to be wrapped by WebCore namespace. Because, ewk files are already using WebCore classes via 'WebCore::'

Sure, my point is that since you already declare and implement the class inside the WebCore namespace, you do not need the `using' clause there too.

[Gyuyoung Kim]

Created attachment 100306 [details]
Patch

[Gyuyoung Kim]

Ok, I remove "using namespace WebCore"

[Raphael Kubo da Costa (:rakuco)]

LGTM.

[Gyuyoung Kim]

Could you anyone review this patch ?

[Gyuyoung Kim]

CC'ing tonikitoo. Sorry for adding CC. However, it was difficult to find proper reviewers.

[WebKit Review Bot]

Comment on attachment 100306 [details]
Patch

Rejecting attachment 100306 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2

Last 500 characters of output:

Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp
patching file Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.h
Hunk #1 succeeded at 112 with fuzz 1.
patching file Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.cpp
patching file Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Antonio Gomes', u'--fo..." exit_code: 1

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

[Gyuyoung Kim]

Created attachment 102226 [details]
Patch

I make a patch with latest WebKit again.

[WebKit Review Bot]

Comment on attachment 102226 [details]
Patch

Clearing flags on attachment: 102226

Committed r91910: <http://trac.webkit.org/changeset/91910>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.