Bug 64064 - [EFL] Add dummy NotificationPresenterClientEfl
Summary: [EFL] Add dummy NotificationPresenterClientEfl
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: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-06 22:48 PDT by Gyuyoung Kim
Modified: 2011-07-27 23:05 PDT (History)
5 users (show)

See Also:


Attachments
Proposed Patch (9.23 KB, patch)
2011-07-06 22:52 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (9.71 KB, patch)
2011-07-11 09:01 PDT, Gyuyoung Kim
tonikitoo: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (9.21 KB, patch)
2011-07-27 21:51 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2011-07-06 22:48:30 PDT
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.
Comment 1 Gyuyoung Kim 2011-07-06 22:52:06 PDT
Created attachment 99938 [details]
Proposed Patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2011-07-07 06:59:29 PDT
> Source/WebKit/efl/WebCoreSupport/NotificationPresenterClientEfl.cpp:28
> +namespace WebCore {

You should not need both `using namespace WebCore' and `namespace WebCore'.
Comment 3 Gyuyoung Kim 2011-07-07 19:31:45 PDT
(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::'
Comment 4 Raphael Kubo da Costa (:rakuco) 2011-07-11 05:57:27 PDT
(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.
Comment 5 Gyuyoung Kim 2011-07-11 09:01:17 PDT
Created attachment 100306 [details]
Patch
Comment 6 Gyuyoung Kim 2011-07-11 09:02:53 PDT
Ok, I remove "using namespace WebCore"
Comment 7 Raphael Kubo da Costa (:rakuco) 2011-07-11 09:08:38 PDT
LGTM.
Comment 8 Gyuyoung Kim 2011-07-17 22:45:34 PDT
Could you anyone review this patch ?
Comment 9 Gyuyoung Kim 2011-07-26 22:32:03 PDT
CC'ing tonikitoo. Sorry for adding CC. However, it was difficult to find proper reviewers.
Comment 10 WebKit Review Bot 2011-07-27 09:45:04 PDT
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
Comment 11 Gyuyoung Kim 2011-07-27 21:51:13 PDT
Created attachment 102226 [details]
Patch

I make a patch with latest WebKit again.
Comment 12 WebKit Review Bot 2011-07-27 23:05:17 PDT
Comment on attachment 102226 [details]
Patch

Clearing flags on attachment: 102226

Committed r91910: <http://trac.webkit.org/changeset/91910>
Comment 13 WebKit Review Bot 2011-07-27 23:05:23 PDT
All reviewed patches have been landed.  Closing bug.