New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 798742 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 796991



Sign in to add a comment

Move kMaximumDeveloperDataSize from PlatformNotificationData to notification.mojom

Project Member Reported by awdf@chromium.org, Jan 3 2018

Issue description

Assuming constants are valid in mojom files, it would be more appropriate for kMaximumDeveloperDataSize to live in

third_party/WebKit/public/platform/modules/notifications/notification.mojom

rather than content/public/common/platform_notification_data.h

(This issue inspired by a comment on https://chromium-review.googlesource.com/c/chromium/src/+/832396 )
 

Comment 1 by awdf@chromium.org, Jan 3 2018

Labels: -Type-Bug Type-Task

Comment 2 by awdf@chromium.org, Apr 12 2018

Blocking: 796991
Status: Started (was: Available)

Comment 3 by awdf@chromium.org, Apr 12 2018

Owner: awdf@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b7146becae09691247fd05c792bc9f84102d464

commit 7b7146becae09691247fd05c792bc9f84102d464
Author: Anita Woodruff <awdf@chromium.org>
Date: Mon Apr 16 22:26:16 2018

[Notifications] Check max data on the renderer-side

- The legacy IPC pathway was checking the max data
size *before* the IPC call, and rejecting if it was
exceeded.

- This patch reinstates the check for the new mojo
pathway, so that we don't risk crashing the renderer
(thanks to verification in struct traits) if max data
is exceeded.

- This involved moving the max data constant to mojo
so it can be accessed from both blink and content.

R=peter@chromium.org

Bug:  798742 , 796991 
Change-Id: I14ae782c9265411da1374beb7ae7312cb265b07e
Reviewed-on: https://chromium-review.googlesource.com/1011602
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551146}
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/content/common/notifications/notification_struct_traits.cc
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/content/public/common/platform_notification_data.h
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/content/renderer/notifications/notification_manager.cc
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/third_party/blink/public/platform/modules/notifications/notification.mojom
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/third_party/blink/renderer/modules/notifications/notification_manager.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b7146becae09691247fd05c792bc9f84102d464

commit 7b7146becae09691247fd05c792bc9f84102d464
Author: Anita Woodruff <awdf@chromium.org>
Date: Mon Apr 16 22:26:16 2018

[Notifications] Check max data on the renderer-side

- The legacy IPC pathway was checking the max data
size *before* the IPC call, and rejecting if it was
exceeded.

- This patch reinstates the check for the new mojo
pathway, so that we don't risk crashing the renderer
(thanks to verification in struct traits) if max data
is exceeded.

- This involved moving the max data constant to mojo
so it can be accessed from both blink and content.

R=peter@chromium.org

Bug:  798742 , 796991 
Change-Id: I14ae782c9265411da1374beb7ae7312cb265b07e
Reviewed-on: https://chromium-review.googlesource.com/1011602
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551146}
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/content/common/notifications/notification_struct_traits.cc
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/content/public/common/platform_notification_data.h
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/content/renderer/notifications/notification_manager.cc
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/third_party/blink/public/platform/modules/notifications/notification.mojom
[modify] https://crrev.com/7b7146becae09691247fd05c792bc9f84102d464/third_party/blink/renderer/modules/notifications/notification_manager.cc

Comment 6 by awdf@chromium.org, May 2 2018

Status: Fixed (was: Started)

Sign in to add a comment