Fix an issue that applicationDidFinishLaunching is not repposted properly.

The applicationDidFinishLaunching should be ran when controller is NOT nil.
Also, make it more clear that the notification is ran right after
the dialog is closed.

Bug: 844487
Change-Id: I8a66c0e79af92234fba9a19dcc36f11f3edb7545
Reviewed-on: https://chromium-review.googlesource.com/1134473Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575308}

chrome/browser/app_controller_mac.h

@@ -193,6 +193,10 @@ bool IsOpeningNewWindow();
 // already existed, notify the AppController of the profile in use.
 void CreateGuestProfileIfNeeded();
 
+// Called when Enterprise startup dialog is close and repost
+// applicationDidFinished notification.
+void EnterpriseStartupDialogClosed();
+
 }  // namespace app_controller_mac
 
 #endif  // CHROME_BROWSER_APP_CONTROLLER_MAC_H_

chrome/browser/app_controller_mac.mm

@@ -197,9 +197,6 @@ bool IsProfileSignedOut(Profile* profile) {
   return has_entry && entry->IsSigninRequired();
 }
 
-// Called when Chrome is launched successfully.
-void LaunchFinished(NSNotification* notify);
-
 }  // namespace
 
 @interface AppController () <HandoffActiveURLObserverBridgeDelegate>
@@ -230,11 +227,6 @@ void LaunchFinished(NSNotification* notify);
 // NTP after all the |urls| have been opened.
 - (void)openUrlsReplacingNTP:(const std::vector<GURL>&)urls;
 
-// Finish Chrome launching. It usually called by -applicationDidFinishLaunching:
-// notification. However, it will be delayed until Chrome browser window is
-// really if there is the EnterpriseStartupDialog blocks Chrome window display.
-- (void)doApplicationDidFinishLaunching:(NSNotification*)notify;
-
 // Whether instances of this class should use the Handoff feature.
 - (BOOL)shouldUseHandoff;
 
@@ -742,7 +734,15 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
 
 // This is called after profiles have been loaded and preferences registered.
 // It is safe to access the default profile here.
-- (void)doApplicationDidFinishLaunching:(NSNotification*)notify {
+- (void)applicationDidFinishLaunching:(NSNotification*)notify {
+  if (g_browser_process->browser_policy_connector()
+          ->machine_level_user_cloud_policy_controller()
+          ->IsEnterpriseStartupDialogShowing()) {
+    // As Chrome is not ready when the Enterprise startup dialog is being shown.
+    // Store the notification as it will be reposted when the dialog is closed.
+    return;
+  }
+
   MacStartupProfiler::GetInstance()->Profile(
       MacStartupProfiler::DID_FINISH_LAUNCHING);
   MacStartupProfiler::GetInstance()->RecordMetrics();
@@ -816,20 +816,6 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
 #endif
 }
 
-- (void)applicationDidFinishLaunching:(NSNotification*)notify {
-  if (g_browser_process->browser_policy_connector()
-          ->machine_level_user_cloud_policy_controller()
-          ->IsEnterpriseStartupDialogShowing()) {
-    // Repost the function with a delayed task if the enterprise startup dialog
-    // is being displayed. Because Chrome is not ready when the dialog is shown.
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, base::BindOnce(&LaunchFinished, notify),
-        base::TimeDelta::FromSeconds(0));
-  } else {
-    [self doApplicationDidFinishLaunching:notify];
-  }
-}
-
 // Helper function for populating and displaying the in progress downloads at
 // exit alert panel.
 - (BOOL)userWillWaitForInProgressDownloads:(int)downloadCount {
@@ -1757,13 +1743,6 @@ void UpdateProfileInUse(Profile* profile, Profile::CreateStatus status) {
   }
 }
 
-void LaunchFinished(NSNotification* notify) {
-  AppController* controller =
-      base::mac::ObjCCastStrict<AppController>([NSApp delegate]);
-  if (controller == nil)
-    [controller doApplicationDidFinishLaunching:notify];
-}
-
 }  // namespace
 
 namespace app_controller_mac {
@@ -1779,4 +1758,15 @@ void CreateGuestProfileIfNeeded() {
       std::string());
 }
 
+void EnterpriseStartupDialogClosed() {
+  AppController* controller =
+      base::mac::ObjCCastStrict<AppController>([NSApp delegate]);
+  if (controller != nil) {
+    NSNotification* notify = [NSNotification
+        notificationWithName:NSApplicationDidFinishLaunchingNotification
+                      object:NSApp];
+    [controller applicationDidFinishLaunching:notify];
+  }
+}
+
 }  // namespace app_controller_mac

chrome/browser/chrome_browser_main.cc

@@ -229,6 +229,7 @@
 #include <Security/Security.h>
 
 #include "base/mac/scoped_nsautorelease_pool.h"
+#include "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/mac/keystone_glue.h"
 #endif  // defined(OS_MACOSX)
 
@@ -756,7 +757,12 @@ bool WaitUntilMachineLevelUserCloudPolicyEnrollmentFinished(
   switch (connector->machine_level_user_cloud_policy_controller()
               ->WaitUntilPolicyEnrollmentFinished()) {
     case RegisterResult::kNoEnrollmentNeeded:
+    case RegisterResult::kEnrollmentSuccessBeforeDialogDisplayed:
+      return true;
     case RegisterResult::kEnrollmentSuccess:
+#if defined(OS_MACOSX)
+      app_controller_mac::EnterpriseStartupDialogClosed();
+#endif
       return true;
     case RegisterResult::kRestartDueToFailure:
       chrome::AttemptRestart();

chrome/browser/policy/cloud/machine_level_user_cloud_policy_browsertest.cc

@@ -427,6 +427,16 @@ IN_PROC_BROWSER_TEST_P(MachineLevelUserCloudPolicyEnrollmentTest, Test) {
   histogram_tester_.ExpectBucketCount(kEnrollmentResultMetrics, expected_result,
                                       1);
   histogram_tester_.ExpectTotalCount(kEnrollmentResultMetrics, 1);
+
+#if defined(OS_MACOSX)
+  // Verify the last mericis of launch is recorded in
+  // applicationDidFinishNotification.
+  if (is_enrollment_token_valid()) {
+    EXPECT_EQ(1u, histogram_tester_
+                      .GetAllSamples("Startup.OSX.DockIconWillFinishBouncing")
+                      .size());
+  }
+#endif
 }
 
 INSTANTIATE_TEST_CASE_P(,

chrome/browser/policy/machine_level_user_cloud_policy_controller.h

@@ -34,9 +34,12 @@ class MachineLevelUserCloudPolicyController {
   enum class RegisterResult {
     kNoEnrollmentNeeded,  // The device won't be enrolled without an enrollment
                           // token.
-    kEnrollmentSuccess,   // The device has been enrolled successfully.
-    kQuitDueToFailure,  // The enrollment has failed or aborted, user choose to
-                        // quit Chrome.
+    kEnrollmentSuccessBeforeDialogDisplayed,  // The enrollment process is
+                                              // finished before dialog
+                                              // displayed.
+    kEnrollmentSuccess,  // The device has been enrolled successfully
+    kQuitDueToFailure,   // The enrollment has failed or aborted, user choose to
+                         // quit Chrome.
     kRestartDueToFailure,  // The enrollment has failed, user choose to restart
   };
 

chrome/browser/policy/machine_level_user_cloud_policy_register_watcher.cc

@@ -44,7 +44,7 @@ RegisterResult MachineLevelUserCloudPolicyRegisterWatcher::
 
   // We are already enrolled successfully.
   if (!token_storage->RetrieveDMToken().empty()) {
-    return RegisterResult::kEnrollmentSuccess;
+    return RegisterResult::kEnrollmentSuccessBeforeDialogDisplayed;
   }
 
   EnterpriseStartupDialog::DialogResultCallback callback = base::BindOnce(

chrome/browser/policy/machine_level_user_cloud_policy_register_watcher_unittest.cc

@@ -151,7 +151,7 @@ class MachineLevelUserCloudPolicyRegisterWatcherTest : public ::testing::Test {
 TEST_F(MachineLevelUserCloudPolicyRegisterWatcherTest,
        NoEnrollmentNeededWithDMToken) {
   storage()->set_dm_token(kDMToken);
-  EXPECT_EQ(RegisterResult::kEnrollmentSuccess,
+  EXPECT_EQ(RegisterResult::kEnrollmentSuccessBeforeDialogDisplayed,
             watcher()->WaitUntilCloudPolicyEnrollmentFinished());
 }