Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(762)

Issue 187803005: Remove uses of the Legacy Notification API and tests on Linux Aura (Closed)

Created:
6 years, 9 months ago by Peter Beverloo
Modified:
6 years, 9 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Visibility:
Public.

Description

Remove uses of the Legacy Notification API and tests on Linux Aura. Blink is removing support for the legacy notification API, and there are a number of Chromium-sided tests depending on it. Remove the tests if they don't make sense anymore, or change them to use the Web Notification API instead. This patch also enables the Notification browser tests for Aura Linux, part of interactive_ui_tests, since they run just fine. It are the Desktop Notification tests which have a dependency on Screen. BUG=348019 R=atwilson@chromium.org, dewittj@chromium.org, kalman@chromium.org, miket@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255599

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -104 lines) Patch
M chrome/browser/extensions/notifications_apitest.cc View 1 chunk +0 lines, -12 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 1 chunk +0 lines, -1 line 0 comments Download
M chrome/test/data/extensions/api_test/activity_log_private/friend/reply.js View 1 2 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/test/data/extensions/api_test/activity_log_private/test/test.js View 1 2 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/test/data/extensions/api_test/notifications/galore/app/controller.js View 1 1 chunk +4 lines, -1 line 0 comments Download
M chrome/test/data/extensions/api_test/notifications/has_not_permission/background.js View 1 chunk +10 lines, -17 lines 0 comments Download
M chrome/test/data/extensions/api_test/notifications/has_permission_prefs/background.js View 1 chunk +6 lines, -14 lines 0 comments Download
D chrome/test/data/extensions/api_test/notifications/no_html/background.js View 1 chunk +0 lines, -20 lines 0 comments Download
D chrome/test/data/extensions/api_test/notifications/no_html/manifest.json View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/test/data/notifications/notification_tester.html View 3 chunks +14 lines, -18 lines 0 comments Download
M chrome/test/data/notifications/notifications_request_function.html View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/notifications/notifications_request_inline.html View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 16 (0 generated)
Peter Beverloo
+atwilson for notification changes. +kalman for chrome/browser/extensions/ RS. This fixes the test failures caused by ...
6 years, 9 months ago (2014-03-06 15:54:49 UTC) #1
not at google - send to devlin
lgtm but FYI +miket since he has more notifications background than me. Where did this ...
6 years, 9 months ago (2014-03-06 16:02:30 UTC) #2
Peter Beverloo
On 2014/03/06 16:02:30, kalman wrote: > Where did this new notifications API come from? Perhaps ...
6 years, 9 months ago (2014-03-06 16:17:05 UTC) #3
Andrew T Wilson (Slow)
lgtm
6 years, 9 months ago (2014-03-06 17:54:48 UTC) #4
Peter Beverloo
The CQ bit was checked by peter@chromium.org
6 years, 9 months ago (2014-03-06 19:56:34 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/peter@chromium.org/187803005/40001
6 years, 9 months ago (2014-03-06 19:58:29 UTC) #6
miket_OOO
LGTM, +dewitt to confirm that this isn't breaking anything in chrome.notifications
6 years, 9 months ago (2014-03-06 20:08:49 UTC) #7
dewittj
sweet! lgtm+
6 years, 9 months ago (2014-03-06 20:48:23 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/peter@chromium.org/187803005/40001
6 years, 9 months ago (2014-03-06 21:05:36 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/peter@chromium.org/187803005/40001
6 years, 9 months ago (2014-03-06 21:59:45 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-07 04:04:21 UTC) #11
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) base_unittests, net_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=276897
6 years, 9 months ago (2014-03-07 04:04:22 UTC) #12
Peter Beverloo
The CQ bit was checked by peter@chromium.org
6 years, 9 months ago (2014-03-07 10:52:22 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/peter@chromium.org/187803005/40001
6 years, 9 months ago (2014-03-07 10:54:13 UTC) #14
Peter Beverloo
The CQ bit was unchecked by peter@chromium.org
6 years, 9 months ago (2014-03-07 10:54:21 UTC) #15
Peter Beverloo
6 years, 9 months ago (2014-03-07 12:48:33 UTC) #16
Message was sent while issue was closed.
Committed patchset #3 manually as r255599 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698