Bug 77969

Summary:

Add web notifications support for WebKitTestRunner and Mac DumpRenderTree

Product:

WebKit

Reporter:

Yael <yael>

Component:

DOM

Assignee:

Jon Lee <jonlee>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gtk-ews, gustavo, gyuyoung.kim, haraken, jberlin, jonlee, mifenton, rakuco, webkit-bug-importer, webkit.review.bot, xan.lopez

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

77968, 79492, 95093, 95099, 95100, 95127, 95154, 95232, 95233, 95234, 95249, 95493, 95546

Bug Blocks:

80472, 81048, 95506, 95507

Attachments:

Description	Flags
1/9 - Update LayoutTest API for web notifications	none
2/9 - Add SPI to retrieve internal IDs for notifications	haraken: review-, gyuyoung.kim: commit-queue-
3/9 - Add SPI to be able to manually set permissions for origins in tests	haraken: review-
4/9 - DRT support	haraken: review-
5/9 - WTR support	haraken: review-, gyuyoung.kim: commit-queue-
6/9 - Support for adding test output for platform events in DRT	haraken: review-
7/9 - Add WK2 support for that output	haraken: review-
8/9 - WTR support for dump output for platform events	haraken: review-
9/9 - Added basic tests in http/tests/notifications showing output from DRT and WTR for web notifications	haraken: review-
All 9 patches as a whole, for EWS	haraken: review-

Yael

Reported 2012-02-07 04:58:17 PST

Add support for testing web notifications.

Attachments
1/9 - Update LayoutTest API for web notifications (41.86 KB, patch) 2012-08-27 01:26 PDT, Jon Lee	no flags	Details Formatted Diff Diff
2/9 - Add SPI to retrieve internal IDs for notifications (17.93 KB, patch) 2012-08-27 01:28 PDT, Jon Lee	haraken: review- gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
3/9 - Add SPI to be able to manually set permissions for origins in tests (13.48 KB, patch) 2012-08-27 01:29 PDT, Jon Lee	haraken: review-	Details Formatted Diff Diff
4/9 - DRT support (25.36 KB, patch) 2012-08-27 01:30 PDT, Jon Lee	haraken: review-	Details Formatted Diff Diff
5/9 - WTR support (29.84 KB, patch) 2012-08-27 01:32 PDT, Jon Lee	haraken: review- gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
6/9 - Support for adding test output for platform events in DRT (13.69 KB, patch) 2012-08-27 01:33 PDT, Jon Lee	haraken: review-	Details Formatted Diff Diff
7/9 - Add WK2 support for that output (51.44 KB, patch) 2012-08-27 01:34 PDT, Jon Lee	haraken: review-	Details Formatted Diff Diff
8/9 - WTR support for dump output for platform events (16.29 KB, patch) 2012-08-27 01:35 PDT, Jon Lee	haraken: review-	Details Formatted Diff Diff
9/9 - Added basic tests in http/tests/notifications showing output from DRT and WTR for web notifications (14.08 KB, patch) 2012-08-27 01:36 PDT, Jon Lee	haraken: review-	Details Formatted Diff Diff
All 9 patches as a whole, for EWS (180.36 KB, patch) 2012-08-27 01:38 PDT, Jon Lee	haraken: review-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Jon Lee

Comment 1 2012-03-13 15:45:21 PDT

*** Bug 79493 has been marked as a duplicate of this bug. ***

Jon Lee

Comment 2 2012-03-13 15:45:48 PDT

<rdar://problem/10357618>

Jon Lee

Comment 3 2012-08-27 01:26:49 PDT

Created attachment 160656 [details] 1/9 - Update LayoutTest API for web notifications

Jon Lee

Comment 4 2012-08-27 01:28:24 PDT

Created attachment 160657 [details] 2/9 - Add SPI to retrieve internal IDs for notifications

Jon Lee

Comment 5 2012-08-27 01:29:25 PDT

Created attachment 160658 [details] 3/9 - Add SPI to be able to manually set permissions for origins in tests

Jon Lee

Comment 6 2012-08-27 01:30:48 PDT

Created attachment 160659 [details] 4/9 - DRT support

Jon Lee

Comment 7 2012-08-27 01:32:09 PDT

Created attachment 160660 [details] 5/9 - WTR support

WebKit Review Bot

Comment 8 2012-08-27 01:32:41 PDT

Attachment 160657 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h:72: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/mac/WebCoreSupport/WebNotificationClient.h:53: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jon Lee

Comment 9 2012-08-27 01:33:16 PDT

Created attachment 160661 [details] 6/9 - Support for adding test output for platform events in DRT

Jon Lee

Comment 10 2012-08-27 01:34:29 PDT

Created attachment 160662 [details] 7/9 - Add WK2 support for that output

Jon Lee

Comment 11 2012-08-27 01:35:19 PDT

Created attachment 160665 [details] 8/9 - WTR support for dump output for platform events

Jon Lee

Comment 12 2012-08-27 01:36:10 PDT

Created attachment 160666 [details] 9/9 - Added basic tests in http/tests/notifications showing output from DRT and WTR for web notifications

Jon Lee

Comment 13 2012-08-27 01:38:50 PDT

Created attachment 160668 [details] All 9 patches as a whole, for EWS

WebKit Review Bot

Comment 14 2012-08-27 01:42:26 PDT

Attachment 160668 [details] did not pass style-queue: Tools/WebKitTestRunner/TestController.h:34: Alphabetical sorting problem. [build/include_order] [4] Tools/DumpRenderTree/mac/MockWebNotificationProvider.h:61: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] Source/WebKit/mac/WebCoreSupport/WebNotificationClient.h:53: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleNotification.cpp:27: You should add a blank line after implementation file's own header. [build/include_order] [4] Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleNotification.cpp:29: Alphabetical sorting problem. [build/include_order] [4] Tools/WebKitTestRunner/WebNotificationProvider.h:29: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h:75: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageNotificationClient.h:50: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageNotificationClient.h:50: The parameter name "origin" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:167: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:167: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:168: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:168: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:169: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:169: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:170: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:170: The parameter name "origin" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:171: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:171: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:172: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:172: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:173: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:173: The parameter name "notification" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:174: The parameter name "page" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h:174: TFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 he parameter name "origin" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 25 in 89 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gyuyoung Kim

Comment 15 2012-08-27 01:43:26 PDT

Comment on attachment 160657 [details] 2/9 - Add SPI to retrieve internal IDs for notifications Attachment 160657 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13608293

Gyuyoung Kim

Comment 16 2012-08-27 01:46:44 PDT

Comment on attachment 160660 [details] 5/9 - WTR support Attachment 160660 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13620021

kov's GTK+ EWS bot

Comment 17 2012-08-27 01:50:16 PDT

Comment on attachment 160657 [details] 2/9 - Add SPI to retrieve internal IDs for notifications Attachment 160657 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13620022

kov's GTK+ EWS bot

Comment 18 2012-08-27 01:55:26 PDT

Comment on attachment 160660 [details] 5/9 - WTR support Attachment 160660 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13609238

Build Bot

Comment 19 2012-08-27 02:00:17 PDT

Comment on attachment 160657 [details] 2/9 - Add SPI to retrieve internal IDs for notifications Attachment 160657 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13615072

Kentaro Hara

Comment 20 2012-08-27 02:07:03 PDT

Please do not upload multiple r? patches at the same time. That's confusing and makes it difficult to comment on individual patches. The best work-around would be: - Create a meta bug and upload one patch that implements everything, so that reviewers can understand what you are going to do. - Create one bug per one sub-patch and set r?, blocking the meta bug. When the first patch is landed, you can upload the second patch etc.

Build Bot

Comment 21 2012-08-27 02:07:36 PDT

Comment on attachment 160660 [details] 5/9 - WTR support Attachment 160660 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13613071

Kentaro Hara

Comment 22 2012-08-27 02:08:37 PDT

Comment on attachment 160656 [details] 1/9 - Update LayoutTest API for web notifications Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 23 2012-08-27 02:08:54 PDT

Comment on attachment 160657 [details] 2/9 - Add SPI to retrieve internal IDs for notifications Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 24 2012-08-27 02:09:12 PDT

Comment on attachment 160660 [details] 5/9 - WTR support Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 25 2012-08-27 02:09:25 PDT

Comment on attachment 160661 [details] 6/9 - Support for adding test output for platform events in DRT Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 26 2012-08-27 02:09:39 PDT

Comment on attachment 160662 [details] 7/9 - Add WK2 support for that output Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 27 2012-08-27 02:09:50 PDT

Comment on attachment 160665 [details] 8/9 - WTR support for dump output for platform events Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 28 2012-08-27 02:10:02 PDT

Comment on attachment 160658 [details] 3/9 - Add SPI to be able to manually set permissions for origins in tests Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 29 2012-08-27 02:10:13 PDT

Comment on attachment 160659 [details] 4/9 - DRT support Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 30 2012-08-27 02:10:25 PDT

Comment on attachment 160666 [details] 9/9 - Added basic tests in http/tests/notifications showing output from DRT and WTR for web notifications Not to show up these patches in the review list, let me r- them for now.

Kentaro Hara

Comment 31 2012-08-27 02:10:52 PDT

Comment on attachment 160668 [details] All 9 patches as a whole, for EWS Not to show up these patches in the review list, let me r- them for now.

Jon Lee

Comment 32 2012-08-27 08:44:33 PDT

(In reply to comment #20) > Please do not upload multiple r? patches at the same time. That's confusing and makes it difficult to comment on individual patches. > > The best work-around would be: > > - Create a meta bug and upload one patch that implements everything, so that reviewers can understand what you are going to do. > > - Create one bug per one sub-patch and set r?, blocking the meta bug. When the first patch is landed, you can upload the second patch etc. All right, I'll split the patches up into smaller bugs.

Jon Lee

Comment 33 2012-08-27 14:47:30 PDT

I ended up doing double duty bringing web notifications support to Mac. There are a couple patches that are common to both test harnesses, so this will act as the master bug for both.

Note You need to log in before you can comment on or make changes to this bug.