Night Light: Fix guest mode not getting the correct geolocation

In guest mode, Night Light used to be initialized from user prefs
before NightLightClient was created, which made it miss getting the
initial value of the ScheduleType. Therefore it never triggered a
request to get the geoposition to update the correct sunset/sunrise
times, and as a result the default 6pm to 6am were used.

This made AutoNightLight always turn on for guest mode after 6pm,
and since it's ephemeral, it doesn't have any cached geolocations.

This CL fixes the order, however, getting the updated geoposition
takes a second, so NL might turn on briefly for a second before
it turns back off once the correct sunset/sunrise times are set.
Hence, this CL also makes sure a stale AutoNightLight notification
will be dismissed without affecting whether it should show again
in the future.

BUG=1106586
TEST=Added a new test.

Change-Id: Ifbf0728ef5362bfbe88d50465943dce9fc0444fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2321719
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792400}

ash/system/night_light/night_light_controller_impl.cc

@@ -989,6 +989,14 @@ void NightLightControllerImpl::OnEnabledPrefChanged() {
   VLOG(1) << "Enable state changed. New state: " << enabled << ".";
   DCHECK(active_user_pref_service_);
 
+  // When there's no valid geolocation, the default sunset/sunrise times are
+  // used, which could lead to Auto Night Light turning on briefly until a valid
+  // geolocation is received. At that point, the Notification will be stale, and
+  // needs to be removed. It doesn't hurt to remove it always, before we update
+  // its state. https://crbug.com/1106586.
+  message_center::MessageCenter::Get()->RemoveNotification(kNotificationId,
+                                                           /*by_user=*/false);
+
   if (enabled && features::IsAutoNightLightEnabled() &&
       GetScheduleType() == kSunsetToSunrise &&
       (is_first_user_init_ ||

ash/system/night_light/night_light_controller_unittest.cc

@@ -1742,6 +1742,40 @@ TEST_F(AutoNightLightTest, Notification) {
   EXPECT_FALSE(controller->GetAutoNightLightNotificationForTesting());
 }
 
+TEST_F(AutoNightLightTest, DismissNotificationOnTurningOff) {
+  GetSessionControllerClient()->UnlockScreen();
+  NightLightControllerImpl* controller = GetController();
+  EXPECT_EQ(NightLightController::kSunsetToSunrise,
+            controller->GetScheduleType());
+
+  // Use a fake geoposition with sunset/sunrise times at 5pm/3am.
+  controller->SetCurrentGeoposition(NightLightController::SimpleGeoposition{
+      kFakePosition2_Latitude, kFakePosition2_Longitude});
+
+  // Simulate reaching sunset.
+  delegate()->SetFakeNow(TimeOfDay(17 * 60));  // Now is 5:00 PM.
+  controller->timer()->FireNow();
+  EXPECT_TRUE(controller->GetEnabled());
+  auto* notification = controller->GetAutoNightLightNotificationForTesting();
+  ASSERT_TRUE(notification);
+  ASSERT_TRUE(notification->delegate());
+
+  // Simulate receiving an updated geoposition with sunset/sunrise times at
+  // 8pm/4am, so now is before sunset. Night Light should turn off, and the
+  // stale notification from above should be removed. However, its removal
+  // should not affect kAutoNightLightNotificationDismissed.
+  controller->SetCurrentGeoposition(NightLightController::SimpleGeoposition{
+      kFakePosition1_Latitude, kFakePosition1_Longitude});
+  EXPECT_FALSE(controller->GetEnabled());
+  EXPECT_FALSE(controller->GetAutoNightLightNotificationForTesting());
+
+  // Simulate reaching next sunset. The notification should still show, since it
+  // was never dismissed by the user.
+  delegate()->SetFakeNow(TimeOfDay(20 * 60));  // Now is 8:00 PM.
+  controller->timer()->FireNow();
+  EXPECT_TRUE(controller->GetEnabled());
+  EXPECT_TRUE(controller->GetAutoNightLightNotificationForTesting());
+}
 TEST_F(AutoNightLightTest, CannotDisableNotificationWhenSessionIsBlocked) {
   EXPECT_TRUE(Shell::Get()->session_controller()->IsUserSessionBlocked());
 

chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc

@@ -160,6 +160,10 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
 #if BUILDFLAG(ENABLE_WAYLAND_SERVER)
   exo_parts_ = ExoParts::CreateIfNecessary();
 #endif
+
+  night_light_client_ = std::make_unique<NightLightClient>(
+      g_browser_process->shared_url_loader_factory());
+  night_light_client_->Start();
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
@@ -200,10 +204,6 @@ void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
 void ChromeBrowserMainExtraPartsAsh::PostBrowserStart() {
   mobile_data_notifications_ = std::make_unique<MobileDataNotifications>();
 
-  night_light_client_ = std::make_unique<NightLightClient>(
-      g_browser_process->shared_url_loader_factory());
-  night_light_client_->Start();
-
   if (chromeos::features::IsAmbientModeEnabled())
     ambient_client_ = std::make_unique<AmbientClientImpl>();
 }