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

Issue 2258673002: Revert of Remove content::NotificationPermissionDispatcher. (Closed)

Created:
4 years, 4 months ago by megjablon
Modified:
4 years, 4 months ago
CC:
abarth-chromium, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, jam, kinuko+watch, mlamouri+watch-notifications_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, Peter Beverloo
Base URL:
https://chromium.googlesource.com/chromium/src.git@permissions_typemaps
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Remove content::NotificationPermissionDispatcher. (patchset #5 id:80001 of https://codereview.chromium.org/2244913002/ ) Reason for revert: Causing failure on Builder WebKit Linux Leak https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/builds/22036 request-permission-detached-context.html 15:25:46.371 7635 worker/0 http/tests/notifications/request-permission-detached-context.html leaked 15:25:46.371 7635 Xlib: extension "RANDR" missing on display ":9". 15:25:46.371 7635 Xlib: extension "RANDR" missing on display ":9". 15:25:46.374 17565 [4/95] http/tests/notifications/request-permission-detached-context.html failed unexpectedly (leak detected: ({"numberOfLiveActiveDOMObjects":[2,6],"numberOfLiveDocuments":[1,3],"numberOfLiveNodes":[4,36],"numberOfLiveResources":[0,3]})) 15:25:46.372 7635 worker/0 http/tests/notifications/request-permission-detached-context.html failed: 15:25:46.372 7635 worker/0 leak detected: ({"numberOfLiveActiveDOMObjects":[2,6],"numberOfLiveDocuments":[1,3],"numberOfLiveNodes":[4,36],"numberOfLiveResources":[0,3]}) Original issue's description: > Remove content::NotificationPermissionDispatcher. > > blink::Notification can make calls to the Mojo PermissionService itself. > > BUG=561879 > > Committed: https://crrev.com/be876eec511985fc0f476628b7b43dc951bd04c5 > Cr-Commit-Position: refs/heads/master@{#412636} TBR=esprehn@chromium.org,haraken@chromium.org,johnme@chromium.org,mvanouwerkerk@chromium.org,peter@chromium.org,reillyg@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=561879 Committed: https://crrev.com/5241216894d534ab30c763375a78ad68e0d155db Cr-Commit-Position: refs/heads/master@{#412678}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+398 lines, -57 lines) Patch
M content/content_renderer.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
A content/renderer/notification_permission_dispatcher.h View 1 chunk +50 lines, -0 lines 0 comments Download
A content/renderer/notification_permission_dispatcher.cc View 1 chunk +62 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.h View 3 chunks +7 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 3 chunks +13 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/modules.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/notifications/DEPS View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/notifications/Notification.h View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/notifications/Notification.cpp View 2 chunks +11 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/notifications/NotificationManager.h View 4 chunks +2 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/modules/notifications/NotificationManager.cpp View 2 chunks +6 lines, -41 lines 0 comments Download
A third_party/WebKit/Source/modules/notifications/NotificationPermissionClient.h View 1 chunk +36 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/modules/notifications/NotificationPermissionClient.cpp View 1 chunk +35 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/NotificationPermissionClientImpl.h View 1 chunk +32 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/NotificationPermissionClientImpl.cpp View 1 chunk +79 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp View 3 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/web.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/public/blink_headers.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebFrameClient.h View 2 chunks +6 lines, -0 lines 0 comments Download
A third_party/WebKit/public/web/modules/notifications/OWNERS View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/public/web/modules/notifications/WebNotificationPermissionCallback.h View 1 chunk +48 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
megjablon
Created Revert of Remove content::NotificationPermissionDispatcher.
4 years, 4 months ago (2016-08-17 22:55:11 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2258673002/1
4 years, 4 months ago (2016-08-17 22:55:44 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-08-17 22:56:57 UTC) #5
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/5241216894d534ab30c763375a78ad68e0d155db Cr-Commit-Position: refs/heads/master@{#412678}
4 years, 4 months ago (2016-08-17 23:16:53 UTC) #7
haraken
LGTM to revert
4 years, 4 months ago (2016-08-17 23:30:58 UTC) #8
Reilly Grant (use Gerrit)
4 years, 4 months ago (2016-08-20 00:27:50 UTC) #9
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2258353002/ by reillyg@chromium.org.

The reason for reverting is: Fixed leak..

Powered by Google App Engine
This is Rietveld 408576698