Sorry I forgot to add more to the observed behavior in bug report :

What happens instead?
I see Chrome profile picker dialog with all profile icons, when selected profile2(original origin of the notification) launches profile 2 with NTP.

Xi, could you take a look please? Should we perhaps forward to the NotificationPlatformBridgeWin in the start-up path prior to the code that launches this dialog?

I will take a look shortly. 

I am not able to repro the issue. I followed the steps -- the profile is always correctly loaded and I never saw the profile picker dialog. 

Maybe capture and upload a video?

Based on offline chat with chengx@ I have retested this on Windows 10 by enabling the notifications on profile 1 and still see the issue. Please find the steps which I followed :


1. Install and launch Chrome 67.0.3393.4 or above 
2. Click on Chrome profile icon on top right side of the screen --> Manage people --> on the dialog click on Add person
3. From profile 2 navigate to https://tests.peter.sh/notification-generator/, click the "Display the notification" button at the bottom. If you are asked to give permission to display notification, just give it. This should be a one-time thing.
4. Click on "Display the notification" to get 2-3 notifications
5. Notification should display at the bottom right of the screen.(with a pinky drunken cat icon in it.)
6. Wait for ~7sec or more so that the notification on desktop disappears.
7. Close Chrome profile 2 by clicking "X" and exit Chrome from Profile 1 from hotdog menu --> exit
8. Click on the message icon on extreme right bottom of windows task bar(similar to message icon) and click on the notifications under Google Chrome.


Expected behavior :
Profile 2 should be launched with test page.


Observed behavior :
Chrome profile picker dialog is showed.







 

Attaching profile picker dialog which I am seeing. I will update the recording of the issue soon.

Deleted: profile picker.png 122 KB

	profile picker.png 122 KB View Download

That's the user manager's account picker, which is started by the StartupBrowserCreator when it can't determine which profile has to be loaded - see the detailed comment in LaunchBrowserForLastProfiles().

A few thoughts:

  - This happens way after CreatePrimaryProfile() in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() where we currently hook in for delivering the notification event. The profile could be loaded there, otherwise we'd have bailed out.

  - Should we make sure that |silent_launch| is set to TRUE in ProcessCmdLineImpl()? The process should be kept alive through a ScopedKeepAlive when the event is being processed. If the developer choses to open a new window, that should then take over in keeping the process alive.

Re: comment 9

The cmd line that launches chrome is totally controlled by us (from notification_helper). The cmd line doesn't have a kSilentLaunch switch appended to it. So |silent_launch| should be irrelevant to this issue, right?

"const bool silent_launch = command_line.HasSwitch(switches::kSilentLaunch);"

I mean the following |silent_launch| variable:

https://cs.chromium.org/chromium/src/chrome/browser/ui/startup/startup_browser_creator.cc?type=cs&q=ProcessCmdLineImpl&sq=package:chromium&l=573

(Which could also be set by the --no-startup-window flag during process start-up.)

When this flag is set, we won't try to open a browsing window by default, which is the code path that triggers the user manager dialog.

A little more detail about CreatePrimaryProfile().

In CreatePrimaryProfile(), GetStartupProfile() is called (chrome_browser_main.cc, line 472), which calls GetStartupProfilePath() (line 978). Profile id is obtained from the launch id here. 

Re: comment 11

If the |silent_launch| is set to true, ProcessCmdLineImpl() returns early https://cs.chromium.org/chromium/src/chrome/browser/ui/startup/startup_browser_creator.cc?type=cs&q=ProcessCmdLineImpl&sq=package:chromium&l=676

So there won't be a call to LaunchBrowserForLastProfiles(), hence no call to ShowUserManagerOnStartup() which shows the user manager dialog. Am I missing anything here?

|silent_launch| is *not* set to TRUE - I'm wondering whether it should be :).

The dialog is shown through LaunchBrowserForLastProfiles(), which is a code path I don't think we need to reach.

Re comment 14:

Currently, we rely on LaunchBrowserForLastProfiles() --> LaunchBrowser() --> lwp.Launch() --> NotificationPlatformBridgeWin::HandleActivation() to process notification. Besides that, if we set this flag to true, no browser will be ever launched by clicking notification. Is this desired?

Base on offline chat with pbommana@, he didn't see this issue when there was only one profile involved in the whole notification workflow (display + activate). So it seems to be a multi-profile issue.

I checked UMA data. Indeed we have some users (not only just pbommana@) who had notification_helper COM server activated and operate fine, but didn't have NotificationPlatformBridgeWin::HandleActivation run to handle the notification. 

I think it is beneficial to log data to UMA for the related code path in startup_browser_creator.cc for further investigation. I am on this now. 

Any other thoughts?

I logged into the machine that pbommana@ has been using. I followed the steps provided in comment #6 and tried both Canary and Dev (as the screenshot is from Dev). However, it just worked fine and the user manager never showed up. 

Interesting...

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

commit 549464264e16338039237a097185fa211d07d4cc
Author: Xi Cheng <chengx@chromium.org>
Date: Fri Apr 20 04:36:56 2018

Log user manager information during windows notification activation

There have been reports that user manager showed up in chrome's startup when
dealing with Windows native notification activation. This CL logs relevant
data to help investigation.

Bug:  833663 
Change-Id: I90a64c7854e25d1a8dc704a452a3c2eddb97c041
Reviewed-on: https://chromium-review.googlesource.com/1019103
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552262}
[modify] https://crrev.com/549464264e16338039237a097185fa211d07d4cc/chrome/browser/ui/startup/startup_browser_creator.cc
[modify] https://crrev.com/549464264e16338039237a097185fa211d07d4cc/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/549464264e16338039237a097185fa211d07d4cc/tools/metrics/histograms/histograms.xml

Tested the issue on chrome version 67.0.3393.4 using Windows-10 with steps mentioned below:
1) Launch Chrome reported version and added second person from Manage people --> on the dialog clicked on Add person
2) From profile 1 navigate to chrome://settings/content/notifications?search=notifications and block notifications.
3) From profile 2 navigate to https://tests.peter.sh/notification-generator/, clicked on 'Allow' click the "Display the notification" button at the bottom
4) Notification got display at the bottom right of the screen, notification on desktop disappears after ~15 sec
5) Closed Chrome profile 2 by clicking "X" and exited Chrome from Profile 1.
6) Click on the message icon on extreme right bottom of windows task bar, didn't observed any notifications on the notifications pane(it is showing as "No new notifications")

Xi Cheng@ Please find the attached screencast for your reference and help us in verifying the fix.

Thanks!

Deleted: 833663.mp4 7.2 MB

	833663.mp4 7.2 MB View Download

Re: comment 18

Thanks for the report!

Could you do following things?
1) Check chrome://flags/#enable-native-notifications. Enable the flag if it has not.
2) Can you try the latest Canary (67.0.340X.X)? I assume you were using Dev channel?
3) Test with and without step 2(the block thing), and see if that makes a difference.

Please find video recording for the issue as attachment.

Deleted: Recording #4.mp4 3.2 MB

	Recording #4.mp4 3.2 MB View Download

Thanks pbommana@ for the video! I can repro it locally now. I didn't know you clicked "exit" rather than "X" to shut down the last browser window.

This is more about profile management. I will look into it shortly. 

The UMA metrics from the past couple days showed that we don't have any other users (<100) see this issue yet. I think it's because clicking "exit" to shut down browser is just uncommon.

There should be no privacy concerns in this bug. The information in the activated notification is just lost, rather than being picked up by a profile chosen from the profile picker.

M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.

I think the likelihood of the issue occurring in the wild is very low. You'd need at least 2 Chrome profiles on the same machine and alternate using them. You'd also need to shut down chrome in a very uncommon 2-step way (clicking the hidden "exit" button) rather than clicking the visible "X". The metric data we have for the past few weeks shows that no users have encountered this issue so far.

Given this, I think this issue should not be a release blocker. We have another discussion thread where we agreed that this issue doesn't block 50/50 rollout.

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

commit 4a4c327a66bf2628085047b311ff9b5ef540e418
Author: Xi Cheng <chengx@chromium.org>
Date: Thu Apr 26 22:35:46 2018

Not show profile picker when chrome's launched by notification activation

In this case, always use |last_used_profile| which contains the profile id
extracted from the notification launch id, rather than |last_opened_profiles|.

Bug:  833663 ,  734095 
Change-Id: I1f8721ba1dcb7b7ab60264dfcae873f664d8121e
Reviewed-on: https://chromium-review.googlesource.com/1031010
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554175}
[modify] https://crrev.com/4a4c327a66bf2628085047b311ff9b5ef540e418/chrome/browser/ui/startup/startup_browser_creator.cc

The CL in comment 25 should fix this issue. I tested it locally.

pbommana@, feel free to test it on your machine. I think this fix should be in Canary next Monday for sure. If you see any issues, let me know; if not, maybe mark the status "Verified".

Tested the issue on chrome version #68.0.3410.0  using Windows-10 with steps mentioned below:

1) Launch Chrome reported version and added second person from Manage people --> on the dialog clicked on Add person
2) From profile 1 navigate to chrome://settings/content/notifications?search=notifications and block notifications.
3) From profile 2 navigate to https://tests.peter.sh/notification-generator/, clicked on 'Allow' click the "Display the notification" button at the bottom
4) Notification got display at the bottom right of the screen, notification on desktop disappears after ~15 sec
5) Closed Chrome profile 2 by clicking "X" and exited Chrome from Profile 1.
6) Click on the message icon on extreme right bottom of windows task bar, didn't observed any notifications on the notifications pane(it is showing as "No new notifications")

@Prudvi: Please find the attached screencast for your reference and help us in verifying the fix.

Thanks!!

Deleted: 833663.mp4 2.6 MB

	833663.mp4 2.6 MB View Download

Re: 27

Please follow the steps in comment 6, rather than that in bug description.

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

commit 5dd94398bc1f0e9750018637b492ce98e085cf89
Author: Xi Cheng <chengx@chromium.org>
Date: Wed May 02 03:16:09 2018

Make kNotificationLaunchId take precedence over kProfileDirectory

... in CreatePrimaryProfile(). This is consistent with the logic in
GetStartupProfilePath() in startup_browser_creator.cc file.

Bug:  734095 ,  833663 
Change-Id: Ic74bf5af947d0ef75006169676ed3bfcad7cde91
Reviewed-on: https://chromium-review.googlesource.com/1037930
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555298}
[modify] https://crrev.com/5dd94398bc1f0e9750018637b492ce98e085cf89/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/5dd94398bc1f0e9750018637b492ce98e085cf89/chrome/browser/ui/startup/startup_browser_creator.cc

Tested the issue using #68.0.3418.0 on Win 10 and Fix is working as expected as per the steps mentioned below.

Steps:
1. Install and Make sure to launch Chrome with --enable-features=NativeNotifications
2. Click on Chrome profile icon on top right side of the screen --> Manage people --> on the dialog click on Add person
3. From profile 1 navigate to chrome://settings/content/notifications?search=notifications and block notifications.
4. From profile 2 navigate to https://tests.peter.sh/notification-generator/, click the "Display the notification" button at the bottom. If you are asked to give permission to display notification, just give it. This should be a one-time thing.
5. Click on "Display the notification"
6. Notification should display at the bottom right of the screen.(with a pinky drunken cat icon in it.)
7. Wait for ~7sec or more so that the notification on desktop disappears.
8. Close Chrome profile 2 by clicking "X" and exit Chrome from Profile 1.
8. Click on the message icon on extreme right bottom of windows task bar(similar to message icon) and click on the notifications under Google 

Please find the screencast, Hence adding Verified labels.

Thanks!!

Deleted: 833663-68.mp4 5.4 MB

	833663-68.mp4 5.4 MB View Download

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

commit c721c6d43cc9a77e4e4557bba52ebe1f78a8c4f8
Author: Xi Cheng <chengx@chromium.org>
Date: Thu May 03 21:22:39 2018

Add StartupBrowserCreatorTest.NotificationLaunchIdDisablesLastOpenProfiles

This covers the code added in crrev/c/1031010.

Bug:  833663 ,  734095 
Change-Id: Iea3502b3543d076d6dc3285bea6e229320a2b980
Reviewed-on: https://chromium-review.googlesource.com/1040739
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555876}
[modify] https://crrev.com/c721c6d43cc9a77e4e4557bba52ebe1f78a8c4f8/chrome/browser/ui/startup/startup_browser_creator.cc
[modify] https://crrev.com/c721c6d43cc9a77e4e4557bba52ebe1f78a8c4f8/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

Re #30:

The behavior in the screencast is EXPECTED (so sorry for the typo in a previous reply). Thanks sandeepkumars@!

As for now, I am pretty sure the profile picker won't show again. If you see other issues in the future, please open new bugs.

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

commit 464ac56cfa314e2823f50be894095963b1683969
Author: Xi Cheng <chengx@chromium.org>
Date: Wed May 23 19:15:12 2018

Retire metric Notifications.Windows.UserManagerShowupStatus

This metric is no longer needed as the issue has been fixed already.

Bug:  833663 ,  734095 
Change-Id: I1a1683370700cdd650e7bdca651c27a2a7a25db4
Reviewed-on: https://chromium-review.googlesource.com/1066998
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561195}
[modify] https://crrev.com/464ac56cfa314e2823f50be894095963b1683969/chrome/browser/ui/startup/startup_browser_creator.cc
[modify] https://crrev.com/464ac56cfa314e2823f50be894095963b1683969/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/464ac56cfa314e2823f50be894095963b1683969/tools/metrics/histograms/histograms.xml

Tested the issue using #68.0.3439.0 on Win 10 and Fix is working as expected as per the steps mentioned below.

Steps:
1. Install and Make sure to launch Chrome with --enable-features=NativeNotifications
2. Click on Chrome profile icon on top right side of the screen --> Manage people --> on the dialog click on Add person
3. From profile 1 navigate to chrome://settings/content/notifications?search=notifications and block notifications.
4. From profile 2 navigate to https://tests.peter.sh/notification-generator/, click the "Display the notification" button at the bottom. If you are asked to give permission to display notification, just give it. This should be a one-time thing.
5. Click on "Display the notification"
6. Notification should display at the bottom right of the screen.(with a pinky drunken cat icon in it.)
7. Wait for ~7sec or more so that the notification on desktop disappears.
8. Close Chrome profile 2 by clicking "X" and exit Chrome from Profile 1.
8. Click on the message icon on extreme right bottom of windows task bar(similar to message icon) and click on the notifications under Google 

Please find the screencast, Hence adding Verified labels.

Thanks!!

Deleted: notifications.mp4 4.0 MB

	notifications.mp4 4.0 MB View Download