Fix some non-aura specific code related to metro startup.

chrome/browser/chrome_process_finder_win.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/chrome_process_finder_win.h"
 
 #include <shellapi.h>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 94 matching lines @@
 NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window,
                                                 bool fast_start) {
   DCHECK(remote_window);
   static const char kSearchUrl[] =
       "http://www.google.com/search?q=%s&sourceid=chrome&ie=UTF-8";
   DWORD process_id = 0;
   DWORD thread_id = GetWindowThreadProcessId(remote_window, &process_id);
   if (!thread_id || !process_id)
     return NOTIFY_FAILED;
 
+#if !defined(USE_AURA)
   if (base::win::IsMetroProcess()) {
     // Interesting corner case. We are launched as a metro process but we
     // found another chrome running. Since metro enforces single instance then
     // the other chrome must be desktop chrome and this must be a search charm
     // activation. This scenario is unique; other cases should be properly
     // handled by the delegate_execute which will not activate a second chrome.
     string16 terms;
     base::win::MetroLaunchType launch = base::win::GetMetroLaunchParams(&terms);
     if (launch != base::win::METRO_SEARCH) {
       LOG(WARNING) << "In metro mode, but and launch is " << launch;
     } else {
       std::string query = EscapeQueryParamValue(UTF16ToUTF8(terms), true);
       std::string url = base::StringPrintf(kSearchUrl, query.c_str());
       SHELLEXECUTEINFOA sei = { sizeof(sei) };
       sei.fMask = SEE_MASK_FLAG_LOG_USAGE;
       sei.nShow = SW_SHOWNORMAL;
       sei.lpFile = url.c_str();
       OutputDebugStringA(sei.lpFile);
       sei.lpDirectory = "";
       ::ShellExecuteExA(&sei);
     }
     return NOTIFY_SUCCESS;
   }
 
   base::win::ScopedHandle process_handle;
   if (base::win::GetVersion() >= base::win::VERSION_WIN8 &&
       base::OpenProcessHandleWithAccess(
           process_id, PROCESS_QUERY_INFORMATION,
-          process_handle.Receive()) &&
-      base::win::IsProcessImmersive(process_handle.Get())) {
-    chrome::ActivateMetroChrome();
+          process_handle.Receive())) {
+    // Receive() causes the process handle to be set in the destructor of the

[gab, 2013/11/05 22:46:55]

This very much surprised me as I expected the object returned by Receive() to
only live in the scope of base::OpenProcessHandleWithAccess()... so I wrote some
code to verify...

class Foo {
 public:
  Foo() { cout << "constructed foo\n"; }
  ~Foo() {cout << "destructed foo\n"; }
};

bool DoSomethingWithFoo(const Foo& foo) {
  cout << "doing the foo dance\n";
  return true;
}

bool DoSomethingElse() {
  cout << "sad in my bar with no foo\n";
  return true;
}

int _tmain(int argc, wchar_t* argv[])
{
  cout << "warming up\n";
  if (DoSomethingWithFoo(Foo()) &&
      DoSomethingElse()) {
    cout << "done\n";
    return 1;
  }
  return 0;
}

.....

And the output was:
warming up
constructed foo
doing the foo dance
sad in my bar with no foo
destructed foo
done

Showing that the scope of Foo() is indeed the entire if evaluation scope, not
only DoSomethingWithFoo()... very weird... I almost feel this is an MSVC bug...
worth trying on a Linux box with clank if someone has one handy I'd say.

[gab, 2013/11/05 22:50:12]

this ^^^

+    // temporary receiver object, which does not happen until after the if
+    // statement is complete.  So IsProcessImmersive() should only be checked
+    // as part of a separate if statement.

[gab, 2013/11/05 22:46:55]

This is pretty bad... a comment should probably be added to
process_handle.Receive()... even better, this should use a different method as I
don't feel this erroneous usage pattern is uncommon...

+    if (base::win::IsProcessImmersive(process_handle.Get()))
+      chrome::ActivateMetroChrome();

[gab, 2013/11/05 22:46:55]

The use case this was meant to handle is that if you launch chrome.exe from the
command-line (and thus bypass delegate_execute.exe's logic to decide Metro vs
Desktop) we still want to rendez-vous with the Metro Chrome (in singleton mode
we have had no choice...).

Now.. in Aura.. if the only active desktop is Ash; should we activate here
instead of opening a native desktop window? We should do whatever
delegate_execute.exe does in the regular case IMO.

   }
+#endif
 
   CommandLine command_line(*CommandLine::ForCurrentProcess());
   command_line.AppendSwitchASCII(
       switches::kOriginalProcessStartTime,
       base::Int64ToString(
           base::CurrentProcessInfo::CreationTime().ToInternalValue()));
 
   if (fast_start)
     command_line.AppendSwitch(switches::kFastStart);
 
@@ skipping 29 matching lines @@
 
   // It is possible that the process owning this window may have died by now.
   if (!::IsWindow(remote_window))
     return NOTIFY_FAILED;
 
   // If the window couldn't be notified but still exists, assume it is hung.
   return NOTIFY_WINDOW_HUNG;
 }
 
 }  // namespace chrome