New issue
Advanced search Search tips

Issue 841475 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

"NotificationHelperLaunchesChrome.ChromeLaunchTest" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, May 9 2018

Issue description

"NotificationHelperLaunchesChrome.ChromeLaunchTest" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyPAsSBUZsYWtlIjFOb3RpZmljYXRpb25IZWxwZXJMYXVuY2hlc0Nocm9tZS5DaHJvbWVMYXVuY2hUZXN0DA.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 
Labels: -Sheriff-Chromium
Owner: chengx@chromium.org
Status: Assigned (was: Untriaged)
Sheriff triage!

chengx@, looks like this test is flaking.  Given it was only recently added, I wonder if it's always been flaky?  Mind taking a look?
Project Member

Comment 2 by chromium...@appspot.gserviceaccount.com, May 11 2018

Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "NotificationHelperLaunchesChrome.ChromeLaunchTest". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyPAsSBUZsYWtlIjFOb3RpZmljYXRpb25IZWxwZXJMYXVuY2hlc0Nocm9tZS5DaHJvbWVMYXVuY2hUZXN0DA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).

Comment 3 by hbos@chromium.org, May 14 2018

Labels: -Sheriff-Chromium
Haven't seen this today, removing sheriff label, feel free to re-add if it keeps happening.
Project Member

Comment 4 by chromium...@appspot.gserviceaccount.com, May 15 2018

Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "NotificationHelperLaunchesChrome.ChromeLaunchTest". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyPAsSBUZsYWtlIjFOb3RpZmljYXRpb25IZWxwZXJMYXVuY2hlc0Nocm9tZS5DaHJvbWVMYXVuY2hUZXN0DA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
The flakiness seems limited to Win10 so far - let me put together a CL that disables the test on Windows (per "flaky tests should be disabled within 30 minutes" sheriffing guidance above :-)
Project Member

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

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

commit 20aa62981af7d622b6d5c8b843cb119d08f95b24
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue May 15 20:20:34 2018

Disable flaky NotificationHelperLaunchesChrome.ChromeLaunchTest on Win.

Bug:  841475 
Change-Id: I55e6008c29008fea3ca24bdca430fa3daa66a0d8
Tbr: chengx@chromium.org
Tbr: peter@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1060165
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558822}
[modify] https://crrev.com/20aa62981af7d622b6d5c8b843cb119d08f95b24/chrome/browser/notifications/notification_helper_launches_chrome_unittest.cc

Labels: -Sheriff-Chromium
Project Member

Comment 8 by chromium...@appspot.gserviceaccount.com, May 16 2018

Labels: Sheriff-Chromium
Detected 18 new flakes for test/step "NotificationHelperLaunchesChrome.ChromeLaunchTest". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyPAsSBUZsYWtlIjFOb3RpZmljYXRpb25IZWxwZXJMYXVuY2hlc0Nocm9tZS5DaHJvbWVMYXVuY2hUZXN0DA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).

Comment 9 by chengx@chromium.org, May 16 2018

Cc: chengx@chromium.org
Labels: OS-Windows
Owner: finnur@chromium.org
Hello Finnur, could yo please take a look of this? I believe this is caused by the code you added below:

::CoAllowSetForegroundWindow(notification_activator.Get(), nullptr));

The error code is: 0x80070005 Access is denied.

You can find these information in the link provided as in comment 8.

We definitely want this test to be re-enabled, so could you fix it and re-enable the test? Maybe just remove the code you added in the test?
Labels: -Sheriff-Chromium
Removing Sheriff-Chromium label again - hopefully #c8 just didn't yet get the effect of disabling the test from #c6.
This is not caused by the CoAllowSetForegroundWindow call, but because Activate should is returning a failure if it fails to set the window to the foreground. It should return S_OK because that is a benign error with a workaround for the user (click the flashing icon in the task bar). 

Fix in review.
https://chromium-review.googlesource.com/c/chromium/src/+/1061414
Project Member

Comment 12 by bugdroid1@chromium.org, May 17 2018

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

commit 31f8df48a7c5bcc8050c636ff0fa60b2bcecbf04
Author: Finnur Thorarinsson <finnur@chromium.org>
Date: Thu May 17 13:23:24 2018

Win Native Notifications: Don't abort if AllowSetForegroundWindow fails.

It may have unwanted side-effects in the Action Center and is really
a benign failure.

Bug:  841475 ,  837796 ,  734095 
Change-Id: I61cc2736742a50de72fd5a5b9c87ae233ff9a5f0
Reviewed-on: https://chromium-review.googlesource.com/1061414
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559508}
[modify] https://crrev.com/31f8df48a7c5bcc8050c636ff0fa60b2bcecbf04/chrome/browser/notifications/notification_helper_launches_chrome_unittest.cc
[modify] https://crrev.com/31f8df48a7c5bcc8050c636ff0fa60b2bcecbf04/chrome/notification_helper/notification_activator.cc

Status: Fixed (was: Assigned)

Sign in to add a comment