| 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: | P2 | 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
Yael
2012-02-07 04:58:17 PST
*** Bug 79493 has been marked as a duplicate of this bug. *** Created attachment 160656 [details]
1/9 - Update LayoutTest API for web notifications
Created attachment 160657 [details]
2/9 - Add SPI to retrieve internal IDs for notifications
Created attachment 160658 [details]
3/9 - Add SPI to be able to manually set permissions for origins in tests
Created attachment 160659 [details]
4/9 - DRT support
Created attachment 160660 [details]
5/9 - WTR support
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.
Created attachment 160661 [details]
6/9 - Support for adding test output for platform events in DRT
Created attachment 160662 [details]
7/9 - Add WK2 support for that output
Created attachment 160665 [details]
8/9 - WTR support for dump output for platform events
Created attachment 160666 [details]
9/9 - Added basic tests in http/tests/notifications showing output from DRT and WTR for web notifications
Created attachment 160668 [details]
All 9 patches as a whole, for EWS
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.
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 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 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 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 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 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. 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 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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
(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. 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. |