[Mac] Rebuild app shims when they fail to dyload Chrome Framework.

apps/app_shim/app_shim_interactive_uitest_mac.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import <Cocoa/Cocoa.h>
 #include <vector>
 
 #include "apps/app_shim/app_shim_handler_mac.h"
 #include "apps/app_shim/app_shim_host_manager_mac.h"
 #include "apps/app_shim/extension_app_shim_handler_mac.h"
 #include "apps/switches.h"
 #include "apps/ui/native_app_window.h"
 #include "base/auto_reset.h"
 #include "base/callback.h"
 #include "base/files/file_path_watcher.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/launch_services_util.h"
+#include "base/mac/mac_util.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/path_service.h"
 #include "base/process/launch.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/test/test_timeouts.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/web_app_mac.h"
@@ skipping 262 matching lines @@
                 andBundleId:bundle_id]);
     [base::mac::ObjCCastStrict<NSRunningApplication>(
         [running_shim objectAtIndex:0]) terminate];
     [ns_observer wait];
 
     EXPECT_FALSE(GetFirstAppWindow());
     EXPECT_FALSE(HasAppShimHost(profile(), app->id()));
   }
 }
 
+// Tests that a 32 bit shim attempting to launch 64 bit Chrome will eventually
+// be rebuilt.
+IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, RebuildShim) {
+  // Get the 32 bit shim.
+  base::FilePath test_data_dir;
+  PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
+  base::FilePath shim_path_32 =

[tapted, 2014/06/11 03:32:43]

Should this actually try to pick a shim that is the inverse of the current
build?

i.e. If this gets compiled for 32-bit the test as it is should just bail out,
since a 32-bit build can't make a 64-bit shim to complete the test.

Bailing out is an option, but we could add a prebuilt 64-bit version and test
someone "upgrading" from a 64-bit chrome to 32-bit, which happens a lot on
dev/canary channels.

[jackhou1, 2014/06/13 09:47:48]

Oh yeah, I forgot to ifdef this out. I don't think testing the 64-to-32 case
would add much, all of the code is the same, it would just test that a 64-bit
shim fails to dyload a 32-bit framework.

+      test_data_dir.Append("app_shim").Append("app_shim_32_bit");
+  EXPECT_TRUE(base::PathExists(shim_path_32));
+
+  // Install test app.
+  const extensions::Extension* app = InstallPlatformApp("minimal");
+
+  // Use WebAppShortcutCreator to create a 64 bit shim.
+  web_app::WebAppShortcutCreator shortcut_creator(
+      web_app::GetWebAppDataDirectory(profile()->GetPath(), app->id(), GURL()),
+      web_app::ShortcutInfoForExtensionAndProfile(app, profile()),
+      extensions::FileHandlersInfo());
+  shortcut_creator.UpdateShortcuts();
+  base::FilePath shim_path = shortcut_creator.GetInternalShortcutPath();
+  NSMutableDictionary* plist_64 = [NSMutableDictionary
+      dictionaryWithContentsOfFile:base::mac::FilePathToNSString(
+          shim_path.Append("Contents").Append("Info.plist"))];
+
+  // Copy 32 bit shim to where it's expected to be.
+  // CopyDirectory doesn't seem to work when copying and renaming in one go.
+  ASSERT_TRUE(base::DeleteFile(shim_path, true));
+  ASSERT_TRUE(base::PathExists(shim_path.DirName()));
+  ASSERT_TRUE(base::CopyDirectory(shim_path_32, shim_path.DirName(), true));
+  ASSERT_TRUE(base::Move(shim_path.DirName().Append(shim_path_32.BaseName()),
+                         shim_path));
+  ASSERT_TRUE(base::PathExists(
+      shim_path.Append("Contents").Append("MacOS").Append("app_mode_loader")));
+
+  // Fix up the plist so that it matches the installed test app.
+  NSString* plist_path = base::mac::FilePathToNSString(
+      shim_path.Append("Contents").Append("Info.plist"));
+  NSMutableDictionary* plist =
+      [NSMutableDictionary dictionaryWithContentsOfFile:plist_path];
+
+  NSArray* keys_to_copy = @[
+    base::mac::CFToNSCast(kCFBundleIdentifierKey),
+    base::mac::CFToNSCast(kCFBundleNameKey),
+    app_mode::kCrAppModeShortcutIDKey,
+    app_mode::kCrAppModeUserDataDirKey
+  ];
+  for (NSString* key in keys_to_copy) {
+    [plist setObject:[plist_64 objectForKey:key]
+              forKey:key];
+  }
+  [plist writeToFile:plist_path
+          atomically:YES];
+
+  base::mac::RemoveQuarantineAttribute(shim_path);
+
+  // Launch the shim, it should start the app and ultimately connect over IPC.
+  // This actually happens in multiple launches of the shim. The first time the
+  // shim will fail and instead launch Chrome with --app-id so that the app
+  // starts. The second time is when Chrome launches the shim in response to an
+  // app starting, this time the shim launches Chrome with --app-shim-error,
+  // which causes Chrome to rebuild the shim. Finally, after rebuilding, Chrome
+  // again launches the shim and expects it to behave normally.
+  ExtensionTestMessageListener launched_listener("Launched", false);
+  WindowedAppShimLaunchObserver observer(app->id());
+
+  CommandLine shim_cmdline(CommandLine::NO_PROGRAM);
+  ASSERT_TRUE(base::mac::OpenApplicationWithPath(
+      shim_path, shim_cmdline, kLSLaunchDefaults, NULL));
+
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+  observer.Wait();
+
+  EXPECT_TRUE(GetFirstAppWindow());
+  EXPECT_TRUE(HasAppShimHost(profile(), app->id()));

[tapted, 2014/06/11 03:32:43]

Is there a non-flaky way to do an EXPECT_FALSE version of this between
launched_listener.Wait and observer.Wait? Or possibly before
launched_listener.Wait by hooking into AppLifetimeMonitor::Observer. I think
that would help verify that the shim launch did actually fail the first time and
the app was launched without a shimhost via --app-shim-error

[jackhou1, 2014/06/13 09:47:49]

It should be non-flaky to do those waits sequentially, there are many sequential
steps on both the UI and FILE threads involved in the rebuild.

+}
+
 }  // namespace apps