@Yoshiki: please confirm the behavior
Thanks!

Any updates? M60 goes stable in 3 weeks.

As I checked on 60.0.3112.52, it doesn't happen. Please feel free to reopen if you still repro the issue.

As per comment 3, Tested the issue on 61.0.3154.0/9736.0.0 dev and Observed that the issue is reproducible..

Steps to Reproduce:
------------------
(1)Sign in  to chrome book with credentials having the sync passphrase
(2)Observe Sync error notification pop-up at the Notification Icon should remain, instead it goes-off immediately.

Attached screen-cast for your reference.Thanks!!

Deleted: 721717.mp4 7.6 MB

	721717.mp4 7.6 MB View Download

Move to M61 as per latest comment 

Will try to get a local repro.

The issue reproduces locally. yoshiki@, please take a look.

Popup disappears after a call with following stack:

#0 0x7fb40fee4ffd base::debug::StackTrace::StackTrace()
#1 0x7fb40fee357c base::debug::StackTrace::StackTrace()
#2 0x7fb4041dd1a3 message_center::MessageCenterImpl::MarkSinglePopupAsShown()
#3 0x7fb4042313cf message_center::MessagePopupCollection::MarkAllPopupsShown()
#4 0x7fb401c96293 ash::WebNotificationTray::HidePopups()
#5 0x7fb4041edee4 message_center::MessageCenterTray::HidePopupBubbleInternal()
#6 0x7fb4041ee0c0 message_center::MessageCenterTray::HidePopupBubble()
#7 0x7fb401c963f4 ash::WebNotificationTray::UpdateAfterShelfAlignmentChange()
#8 0x7fb401c35722 ash::StatusAreaWidget::UpdateAfterShelfAlignmentChange()
#9 0x7fb401b43522 ash::ShelfWidget::OnShelfAlignmentChanged()
#10 0x7fb401b0d534 ash::Shelf::SetAlignment()
#11 0x7fb401b1e379 ash::ShelfController::SetAlignment()
#12 0x7fb400519c7c ash::mojom::ShelfControllerStubDispatch::Accept()
#13 0x7fb401b22d43 ash::mojom::ShelfControllerStub<>::Accept()
#14 0x7fb40e652496 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#15 0x7fb40e6510d1 mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept()
#16 0x7fb40e64f432 mojo::FilterChain::Accept()
#17 0x7fb40e654daf mojo::InterfaceEndpointClient::HandleIncomingMessage()
#18 0x7fb40e66ca55 mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#19 0x7fb40e66a29c mojo::internal::MultiplexRouter::ProcessTasks()
#20 0x7fb40e66da29 mojo::internal::MultiplexRouter::LockAndCallProcessTasks()
#21 0x7fb40e676a2f _ZN4base8internal13FunctorTraitsIMN4mojo8internal15MultiplexRouterEFvvEvE6InvokeIRK13scoped_refptrIS4_EJEEEvS6_OT_DpOT0_
#22 0x7fb40e6769a4 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN4mojo8internal15MultiplexRouterEFvvEJRK13scoped_refptrIS6_EEEEvOT_DpOT0_
#23 0x7fb40e676950 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo8internal15MultiplexRouterEFvvEJ13scoped_refptrIS5_EEEEFvvEE7RunImplIRKS7_RKNSt3__15tupleIJS9_EEEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#24 0x7fb40e67689c _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo8internal15MultiplexRouterEFvvEJ13scoped_refptrIS5_EEEEFvvEE3RunEPNS0_13BindStateBaseE
#25 0x7fb40fe93251 _ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv
#26 0x7fb40fee995d base::debug::TaskAnnotator::RunTask()
#27 0x7fb40ff9b2de base::MessageLoop::RunTask()
#28 0x7fb40ff9b547 base::MessageLoop::DeferOrRunPendingTask()
#29 0x7fb40ff9c2ca base::MessageLoop::DoWork()
#30 0x7fb40ffa4473 base::MessagePumpLibevent::Run()
#31 0x7fb40ff9ac66 base::MessageLoop::Run()
#32 0x7fb41004b067 base::RunLoop::Run()
#33 0x562d105dfad3 ChromeBrowserMainParts::MainMessageLoopRun()
#34 0x7fb409aeb48b content::BrowserMainLoop::RunMainMessageLoopParts()
#35 0x7fb409af31e5 content::BrowserMainRunnerImpl::Run()
#36 0x7fb409ae014b content::BrowserMain()
#37 0x7fb40b597789 content::RunNamedProcessTypeMain()
#38 0x7fb40b59a117 content::ContentMainRunnerImpl::Run()
#39 0x7fb40b59537d content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#40 0x7fb4107dba4c service_manager::Main()
#41 0x7fb40b5966db content::ContentMain()
#42 0x562d0dfcf2ac ChromeMain
#43 0x562d0dfcf1a2 main
#44 0x7fb3f6bfdf45 __libc_start_main
#45 0x562d0dfcf084 <unknown>

not a regression and not RBS.

I bisected this issue, the change that introduced it is https://crrev.com/2807693002.

Mohsen, could you take a look?
The problem is that previously notification was dismissed after a few seconds from message_center::PopupTimersController::TimerFinished() while after your change it is being dismissed immediately from ash::WebNotificationTray::UpdateAfterShelfAlignmentChange().

Repro steps:
- Enable sync passphrase encryption on account (chrome://settings/syncSetup)
- Sign in into account from fresh profile on chromeos

Expected behavior:
- Passphrase notification is displayed for several seconds after sign-in.

Actual behavior:
- Passphrase notification is hidden immediately.

I can't repro this on ToT (63.0.3215.0). What I see as that the notification bubble is never shown rather than being shown and then dismissed immediately. The notification button, however, shows 1 and the notification is there in the message center. The behavior is the same whether I try with an existing user or add a new user. Can you please check the latest version and confirm that you can repro?

The notification should be shown for a few seconds after signing in to get user's attention. See expected.mp4 video. I believe it is being shown, I'll need to spend some time to grab the stack of where it happens, but it is getting quickly dismissed on the stack I posted in #8.

There is an interesting comment in WebNotificationTray::UpdateAfterShelfAlignmentChange: // Destroy any existing bubble so that it will be rebuilt correctly. [1]

I guess it doesn't rebuild notifications for some reason.

[1] https://cs.chromium.org/chromium/src/ash/system/web_notification/web_notification_tray.cc?type=cs&q=UpdateAfterShelfAlignmentChange&sq=package:chromium&l=400

From what I tested the notification is not shown at all and I can't get the stack trace mentioned in c#8. I even commented the code in WebNotificationTray::UpdateAfterShelfAlignmentChange() and behavior was the same.

Mohsen: I prepared more detailed repro steps. Could you follow them and see how behavior diverges on your machine:

In regular Chrome:
- Enable passphrase encryption: 
  chrome://settings/syncSetup => 
  "Encrypt synced data with your own sync passphrase" => 
  set and confirm passphrase =>
  Save

- Verify custom passphrase is enabled:
  chrome://sync-internals =>
  "Passphrase Type" should be PassphraseType::CUSTOM_PASSPHRASE

On dev machine:
- Checkout chromium at ToT
- Add log statements:
  - sync_service_crypto.cc:253 (SyncServiceCrypto::OnPassphraseRequired): called when sync receives a message from server indicating that user needs to enter passphrase.
  - sync_error_notifier_ash.cc:168 (SyncErrorNotifier::OnErrorChanged): Called when passphrase notification is displayed.
  - message_center_impl.cc (MessageCenterImpl::MarkSinglePopupAsShown): Called from WebNotificationTray::UpdateAfterShelfAlignmentChange. Hides passphrase notification.
- Build chrome for ChromeOS: gn args contains target_os="chromeos" 

- Run from clean profile dir: chrome --user-data-dir=TestProfile --login-manager
- Sign-in with account from above steps
- Check for output from log statements

In my repro I first see log from OnPassphraseRequired(), then from OnErrorChanged() and then immediately from MarkSinglePopupAsShown() which hides notification.

To ensure that sync passphrase is required check chrome://sync-internals =>
  "Passphrase Type" should be PassphraseType::CUSTOM_PASSPHRASE

Thanks pavely@. I followed your repro steps. I didn't get any logs from MessageCenterImpl::MarkSinglePopupAsShown() which matches the behavior I described previously; i.e., no notification is shown at all!

Here is what this bug is about:
- Sync component notices something that is worth displaying to user (You need to enter passphrase")
- I expect this message to be displayed as it was before (see screencast in expected.mp4 )
- It isn't.

Log from SyncServiceCrypto::OnPassphraseRequired indicates that sync noticed condition that's worth showing as popup notification.
Log from SyncErrorNotifier::OnErrorChanged indicates that sync attempted to add such popup notification.
This notification is not being displayed.

The absence of call to MessageCenterImpl::MarkSinglePopupAsShown doesn't make this problem resolved, it just means that maybe from the time I got this repro something else has changed in the code and now notification is getting hidden elsewhere.

I am not familiar with ui/notifications/message_center code, I'm just trying to find someone who can confirm that the problem exists and fix it.

On ToT popup notification is being hidden from MessageCenterImpl::OnBlockingStateChanged (call to MarkSinglePopupAsShown). If I comment this code then it hits WebNotificationTray::UpdateAfterShelfAlignmentChange which calls MessageCenterTray::HidePopupBubble.

mohsen@, friendly ping for you. This is definitely a bug; the expected behavior is that once you sign into CrOS with an account that has a sync passphrase, we show the notification for several seconds.

Can you please provide an update or help point us to the right person to look at this issue?

+Omri who also may know who the right POC is for helping address this bug

OK. So there are two issues here:
 1) Originally, the notification was shown but immediately got hidden;
 2) Recently, the notification is not shown at all (it is blocked before issue (1) hides it).

Issue (1) seems to be caused by r465918 (my change).

According to my bisect, issue (2) seems to be caused by r505685.

For now, assigning to xiyuan@ to fix the issue caused by r505685. If it was fixed and issue (1) is still present, please assign back to me to fix it.

I could image the problem caused by my CL is the mixed sync and async calls. My CL replaces SessionManager calls with ash::SessionController equivalents for session state checking. There is a subtle difference that SessionManager state change is synchronous while ash::SessionController session state is asynchronously changed via mojo.

It looks like SyncErrorNotifier calls message center synchronously (via NotificationUIManager::Add [1]) hence its notification is blocked because it is created almost immediately after session state change and ash session state is not updated yet.

estate@, what would be the right fix? Migrate SyncErrorNotifier to use NotificationDisplayService?

[1]: https://cs.chromium.org/chromium/src/chrome/browser/sync/sync_error_notifier_ash.cc?rcl=3e17497f3579fc221310f9452be1129e1f9103f0&l=166

While we should make that migration, I don't think it would fix this bug because NDS is still synchronous (for now). What happens when you comment out the NotifyBlockingStateChanged call?

Looks like the problem is not caused by the mixed sync/async code path.

The notification is marked as shown when the blocker state changes. For the login scenario, there are a couple of blocks in play: LoginStateNotificationBlocker and InactiveUserNotificationBlocker. When one of them changes state while the other one still blocks the popup, the notification is marked as shown [1]. Even if the notification is never being displayed as a popup (because it was blocked), it is marked as shown.

Think the intention of MarkSinglePopupAsShown() is to hide the popups that were displayed as popups and should be hidden now, not the ones that has been blocked all along and never being shown as popup. 

[1]: https://cs.chromium.org/chromium/src/ui/message_center/message_center_impl.cc?rcl=eca759013e8ff178096d8032ece30127d68fa84e&l=115-121

Proposed CL: https://chromium-review.googlesource.com/c/chromium/src/+/773280

Two problems cause the regression:
- Notification blocker state change hides a never displayed pop-up when it is blocked, as in #23. Fixed by changing message center logic to only mark already display pop-ups as shown;

- Shelf initial state change (alignment change from BOTTOM_LOCKED -> BOTTOM when going from login screen to user session), as in #8. Fixed by deferring the pop-up creation until ash has initialized the user PrefService and Shelf finishes the intialization;

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

commit c7388ba52388421e91c113ed807dec16b830c45b
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Mon Nov 20 23:11:25 2017

Fix missing sync error notification pop-up

- Only MarkSinglePopupAsShown of already displayed pup-ups;
- Make LoginStateNotificationBlocker to block notifications until the
  active user PrefService is initialized. This allows Shelf to apply
  user prefs first before allowing pop-ups to be created. So that
  WebNotificationTray does not HidePopupBubble when user PrefService
  is initialized later after session state changes to active;

Bug:  721717 
Change-Id: I93af5c701b37c172dc85980c74dc0b0d3bfa5ccc
Reviewed-on: https://chromium-review.googlesource.com/773280
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517976}
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ash/session/test_session_controller_client.cc
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ash/session/test_session_controller_client.h
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ash/system/web_notification/login_state_notification_blocker.cc
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ash/system/web_notification/login_state_notification_blocker.h
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ash/system/web_notification/login_state_notification_blocker_unittest.cc
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/chrome/browser/notifications/login_state_notification_blocker_chromeos_browsertest.cc
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ui/message_center/message_center_impl.cc
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ui/message_center/message_center_impl_unittest.cc
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ui/message_center/notification_list.cc
[modify] https://crrev.com/c7388ba52388421e91c113ed807dec16b830c45b/ui/message_center/notification_list.h

Fixed in M64.

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

commit 22b2de53df94c085569b43adac93e41622f18908
Author: Takeshi Yoshino <tyoshino@chromium.org>
Date: Tue Nov 21 01:31:36 2017

Revert "Fix missing sync error notification pop-up"

This reverts commit c7388ba52388421e91c113ed807dec16b830c45b.

Reason for revert: Suspected to have broken tests

Original change's description:
> Fix missing sync error notification pop-up
> 
> - Only MarkSinglePopupAsShown of already displayed pup-ups;
> - Make LoginStateNotificationBlocker to block notifications until the
>   active user PrefService is initialized. This allows Shelf to apply
>   user prefs first before allowing pop-ups to be created. So that
>   WebNotificationTray does not HidePopupBubble when user PrefService
>   is initialized later after session state changes to active;
> 
> Bug:  721717 
> Change-Id: I93af5c701b37c172dc85980c74dc0b0d3bfa5ccc
> Reviewed-on: https://chromium-review.googlesource.com/773280
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#517976}

TBR=xiyuan@chromium.org,stevenjb@chromium.org,estade@chromium.org

Change-Id: I65c810c7d4adf8b8e8e938214a2d7a51675a4ce6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  721717 
Reviewed-on: https://chromium-review.googlesource.com/780124
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Takeshi Yoshino <tyoshino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518032}
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ash/session/test_session_controller_client.cc
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ash/session/test_session_controller_client.h
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ash/system/web_notification/login_state_notification_blocker.cc
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ash/system/web_notification/login_state_notification_blocker.h
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ash/system/web_notification/login_state_notification_blocker_unittest.cc
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/chrome/browser/notifications/login_state_notification_blocker_chromeos_browsertest.cc
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ui/message_center/message_center_impl.cc
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ui/message_center/message_center_impl_unittest.cc
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ui/message_center/notification_list.cc
[modify] https://crrev.com/22b2de53df94c085569b43adac93e41622f18908/ui/message_center/notification_list.h

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

commit 75edba483a8c0a577db51cfd92155fdec0befe85
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue Nov 21 21:47:19 2017

Reland "Fix missing sync error notification pop-up"

- Only MarkSinglePopupAsShown of already shown pup-ups;
- Make LoginStateNotificationBlocker to defer showing notifications
  until the active user PrefService is initialized. This allows
  Shelf to apply user prefs first so that WebNotificationTray does
  not HidePopupBubble when user PrefService is initialized sometime
  later after session state changes to active;

Bug:  721717 
Change-Id: I86e9978be7b56e92ce847f0086a5fa2e4f6b72db
Reviewed-on: https://chromium-review.googlesource.com/782359
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518406}
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ash/session/test_session_controller_client.cc
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ash/session/test_session_controller_client.h
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ash/system/web_notification/login_state_notification_blocker.cc
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ash/system/web_notification/login_state_notification_blocker.h
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ash/system/web_notification/login_state_notification_blocker_unittest.cc
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/chrome/browser/notifications/login_state_notification_blocker_chromeos_browsertest.cc
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ui/message_center/message_center_impl.cc
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ui/message_center/message_center_impl_unittest.cc
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ui/message_center/notification_list.cc
[modify] https://crrev.com/75edba483a8c0a577db51cfd92155fdec0befe85/ui/message_center/notification_list.h