[Mac] Re-enable AppShimInteractiveTest.Launch.

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"
@@ skipping 35 matching lines @@
   base::AutoReset<bool> auto_reset_;
 
   DISALLOW_COPY_AND_ASSIGN(AppShimInteractiveTest);
 };
 
 // Watches for changes to a file. This is designed to be used from the the UI
 // thread.
 class WindowedFilePathWatcher
     : public base::RefCountedThreadSafe<WindowedFilePathWatcher> {
  public:
-  WindowedFilePathWatcher(const base::FilePath& path) : observed_(false) {
+  WindowedFilePathWatcher(const base::FilePath& path)
+      : path_(path), observed_(false) {
     content::BrowserThread::PostTask(
         content::BrowserThread::FILE,
         FROM_HERE,
-        base::Bind(&WindowedFilePathWatcher::Watch, this, path));
+        base::Bind(&WindowedFilePathWatcher::Watch, this, path.DirName()));
   }
 
   void Wait() {
     if (observed_)
       return;
 
     run_loop_.reset(new base::RunLoop);
     run_loop_->Run();
   }
 
  protected:
   friend class base::RefCountedThreadSafe<WindowedFilePathWatcher>;
   virtual ~WindowedFilePathWatcher() {}
 
   void Watch(const base::FilePath& path) {

[tapted, 2014/09/23 06:28:41]

I don't think the function argument here is needed any more (ie call
Watch(path_.DirName().. below). It does raise the question about whether it's
safe to set it in the UI thread and then read it immediately, here. But a read
is already happening in Observe(..), and I think it's nicer at least to
construct and destroy on the same thread rather than try to pass strings around
(they're probably reference-counted between threads anyway).

[jackhou1, 2014/09/23 07:34:07]

Done.

     watcher_.Watch(
         path, false, base::Bind(&WindowedFilePathWatcher::Observe, this));
   }
 
   void Observe(const base::FilePath& path, bool error) {
-    content::BrowserThread::PostTask(
-        content::BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(&WindowedFilePathWatcher::StopRunLoop, this));
+    DCHECK(!error);
+    if (base::PathExists(path_)) {
+      content::BrowserThread::PostTask(

[tapted, 2014/09/23 06:28:41]

watcher_->Cancel() before this? Not entirely sure how safe it is if another file
is created in path_.DirName() after this triggers.

[jackhou1, 2014/09/23 07:34:06]

Done.

+          content::BrowserThread::UI,
+          FROM_HERE,
+          base::Bind(&WindowedFilePathWatcher::StopRunLoop, this));
+    }
   }
 
   void StopRunLoop() {
     observed_ = true;
     if (run_loop_.get())
       run_loop_->Quit();
   }
 
  private:
+  base::FilePath path_;

[tapted, 2014/09/23 06:28:41]

nit: const?

[jackhou1, 2014/09/23 07:34:06]

Done.

   base::FilePathWatcher watcher_;
   bool observed_;
   scoped_ptr<base::RunLoop> run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(WindowedFilePathWatcher);
 };
 
 // Watches for an app shim to connect.
 class WindowedAppShimLaunchObserver : public apps::AppShimHandler {
  public:
@@ skipping 103 matching lines @@
 
   runLoop_.reset(new base::RunLoop);
   runLoop_->Run();
 }
 
 @end
 
 namespace apps {
 
 // Shims require static libraries http://crbug.com/386024.
-// This test is flaky on OSX. http://crbug.com/415422
-#if defined(COMPONENT_BUILD) || defined(OS_MACOSX)
+#if defined(COMPONENT_BUILD)
 #define MAYBE_Launch DISABLED_Launch
 #define MAYBE_RebuildShim DISABLED_RebuildShim
 #else
 #define MAYBE_Launch Launch
 #define MAYBE_RebuildShim RebuildShim
 #endif
 
 // Test that launching the shim for an app starts the app, and vice versa.
 // These two cases are combined because the time to run the test is dominated
 // by loading the extension and creating the shim.
@@ skipping 11 matching lines @@
 
   // Create the internal app shim by simulating an app update. FilePathWatcher
   // is used to wait for file operations on the shim to be finished before
   // attempting to launch it. Since all of the file operations are done in the
   // same event on the FILE thread, everything will be done by the time the
   // watcher's callback is executed.
   scoped_refptr<WindowedFilePathWatcher> file_watcher =
       new WindowedFilePathWatcher(shim_path);
   web_app::UpdateAllShortcuts(base::string16(), profile(), app);
   file_watcher->Wait();
+  ASSERT_TRUE(base::PathExists(shim_path));
   NSString* bundle_id = GetBundleID(shim_path);
 
   // Case 1: Launch the shim, it should start the app.
   {
     ExtensionTestMessageListener launched_listener("Launched", false);
     CommandLine shim_cmdline(CommandLine::NO_PROGRAM);
     shim_cmdline.AppendSwitch(app_mode::kLaunchedForTest);
     ProcessSerialNumber shim_psn;
     ASSERT_TRUE(base::mac::OpenApplicationWithPath(
         shim_path, shim_cmdline, kLSLaunchDefaults, &shim_psn));
@@ skipping 130 matching lines @@
   // the shim is rebuilt.
   WindowedAppShimLaunchObserver(app->id()).Wait();
 
   EXPECT_TRUE(GetFirstAppWindow());
   EXPECT_TRUE(HasAppShimHost(profile(), app->id()));
 }
 
 #endif  // defined(ARCH_CPU_64_BITS)
 
 }  // namespace apps