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

Issue 836080 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

cheets_MediaProjectionPermissionsTest failing

Project Member Reported by kinaba@chromium.org, Apr 24 2018

Issue description

Originally filed as the internal ARC++ bug (b/78473558), but since it turned out to be an issue in Chromium code, moving to the public tracker.


https://stainless.corp.google.com/search?view=matrix&row=test&col=build&first_date=2017-01-19&last_date=2018-04-23&test=cheets_MediaProjectionPermissionsTest&exclude_cts=true&exclude_not_run=false&exclude_non_release=true&exclude_au=true&exclude_acts=true&exclude_retried=true&exclude_non_production=true

Good: R68-10580.0.0
Bad: R68-10582.0.0

Good: R67-10575.0.0
Bad: R67-10575.2.0



It started failing after native notifications" enabled,
https://chromium.googlesource.com/chromium/src/+/7ec6da3c01779a2db4edaca906cc240f4a5b9327

because getVisibleNotification() started failing to find the notification
https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/autotest_private/autotest_private_api.cc?type=cs&q=getVisibleNotification&sq=package:chromium&l=424

apparently because it's failing to deserialize its icon via mojo:
...
[15410:15410:0414/105718.323751:ERROR:notification_struct_traits.cc(176)] Couldn't find icon: kNotificationScreenshareIcon
[15410:15410:0414/105718.325349:ERROR:validation_errors.cc(87)] Invalid message: VALIDATION_ERROR_DESERIALIZATION_FAILED
[15410:15410:0414/105718.438498:ERROR:notification_struct_traits.cc(176)] Couldn't find icon: kNotificationScreenshareIcon
[15410:15410:0414/105718.440276:ERROR:validation_errors.cc(87)] Invalid message: VALIDATION_ERROR_DESERIALIZATION_FAILED
[15410:15410:0414/105718.563950:ERROR:notification_struct_traits.cc(176)] Couldn't find icon: kNotificationScreenshareIcon
[15410:15410:0414/105718.564070:ERROR:validation_errors.cc(87)] Invalid message: VALIDATION_ERROR_DESERIALIZATION_FAILED
[15410:15410:0414/105718.680435:ERROR:notification_struct_traits.cc(176)] Couldn't find icon: kNotificationScreenshareIcon
...

because the icon is not registered.
https://cs.chromium.org/chromium/src/ui/message_center/public/mojo/notification_struct_traits.cc?q=notification_struct_traits.cc&sq=package:chromium&dr&l=176
 
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 25 2018

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

commit 8af66c3412aa4dea4388e6d9b25b512c74a17099
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Wed Apr 25 14:15:42 2018

ash: Do not return vector icons from message center GetActiveNotifications.

The vector icon field causes Mojo de-serialization error for unregistered
icons, and anyway they are not used by the caller (autotest_private API.)

Bug:  836080 
Test: cheets_MediaProjectionPermissionsTest
Change-Id: Id7f680fafbe91c21e42dce794601deef5e5d8ac2
Reviewed-on: https://chromium-review.googlesource.com/1025243
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Kazuhiro Inaba <kinaba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553545}
[modify] https://crrev.com/8af66c3412aa4dea4388e6d9b25b512c74a17099/ash/message_center/message_center_controller.cc

Labels: Merge-Request-67
Confirmed the test is indeed fixed on the bots after the Chrome roll, on the recent canary builds.

Adding M67 merge request.
The change affects only tests. Production code behavior won't be affected.
Which means we don't strictly need the CL, but without the change, we can never detect any future regressions on the feature on the m67 branch, which is scary.
Project Member

Comment 4 by sheriffbot@chromium.org, May 1 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 Chrome OS. Thanks for the testing and context in #3 :-)
Project Member

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

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df9074f856b0fb4ccb5aabe66105bf5fe9c19943

commit df9074f856b0fb4ccb5aabe66105bf5fe9c19943
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Wed May 02 00:21:40 2018

ash: Do not return vector icons from message center GetActiveNotifications.

The vector icon field causes Mojo de-serialization error for unregistered
icons, and anyway they are not used by the caller (autotest_private API.)

Bug:  836080 
Test: cheets_MediaProjectionPermissionsTest
Change-Id: Id7f680fafbe91c21e42dce794601deef5e5d8ac2
Reviewed-on: https://chromium-review.googlesource.com/1025243
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Kazuhiro Inaba <kinaba@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553545}(cherry picked from commit 8af66c3412aa4dea4388e6d9b25b512c74a17099)
Reviewed-on: https://chromium-review.googlesource.com/1038923
Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#431}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/df9074f856b0fb4ccb5aabe66105bf5fe9c19943/ash/message_center/message_center_controller.cc

Status: Fixed (was: Started)

Sign in to add a comment