Refactor ShellDesktopController and ShellNativeAppWindow to allow for non-aura implementations.

extensions/shell/browser/shell_app_window_client.cc

Index: extensions/shell/browser/shell_app_window_client.cc
diff --git a/extensions/shell/browser/shell_app_window_client.cc b/extensions/shell/browser/shell_app_window_client.cc
index 293f58b91c7dc427f5c2bc6f0638ef2a2daafaad..08ac1c58b1c1edefa713ab2c553da16d5e4ee904 100644
--- a/extensions/shell/browser/shell_app_window_client.cc
+++ b/extensions/shell/browser/shell_app_window_client.cc
@@ -8,7 +8,10 @@
 
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/shell/browser/desktop_controller.h"
-#include "extensions/shell/browser/shell_native_app_window.h"
+
+#if defined(USE_AURA)
+#include "extensions/shell/browser/shell_native_app_window_aura.h"
+#endif
 
 namespace extensions {
 
@@ -24,15 +27,17 @@ AppWindow* ShellAppWindowClient::CreateAppWindow(
   return DesktopController::instance()->CreateAppWindow(context, extension);
 }
 
+#if defined(USE_AURA)

[James Cook, 2014/11/11 18:23:52]

Are you sure you want to ifdef out the whole function, rather than having it
return nullptr on non-aura?

[Yoyo Zhou, 2014/11/15 01:05:44]

Yes, this is confusing: I can't include Obj-C from a .cc file, which
ShellNativeAppWindowMac is going to be. So the mac version of this function has
to be defined in a .mm file. I could make a separate _aura.cc file for this
function, but it seemed like overkill; on the other hand, it'd be more
consistent. Any preference?

[James Cook, 2014/11/17 17:48:36]

I don't understand. Are you saying you would create a
shell_app_window_client_aura.cc vs. shell_app_window_client_mac.cc?  Or that the
header for shell_native_app_window_mac.h is going to have stuff in it that you
can't #include here?

If the latter, how do we handle that elsewhere?  Can we do pointer-to-impl and
have the impl in the .mm class?

[Yoyo Zhou, 2014/11/17 19:35:55]

In my mac build, I have a shell_app_window_client_mac.mm (no separate .h) which
defines this function. So on Mac, that file gets picked up, and this ifdef hides
the definition here. On Linux/Win with Aura, that file doesn't get picked up and
this ifdef is enabled.

My alternative suggestion is to still keep the common header file, but just
implement this function in shell_app_window_client_aura.cc. I don't know exactly
what the best practice is for this.

[James Cook, 2014/11/17 20:54:22]

I think separate files is best practice.

You could also have two entirely separate ShellAppWindowClientMac and
ShellAppWindowClientAura classes and a single factory function to create the one
you want. That's also common practice.

[Yoyo Zhou, 2014/11/17 23:55:26]

I moved this function  to a separate file.

 NativeAppWindow* ShellAppWindowClient::CreateNativeAppWindow(
       AppWindow* window,
       AppWindow::CreateParams* params) {
   ShellNativeAppWindow* native_app_window =
-      new ShellNativeAppWindow(window, *params);
+      new ShellNativeAppWindowAura(window, *params);
   DesktopController::instance()->AddAppWindow(
       native_app_window->GetNativeWindow());
   return native_app_window;
 }
+#endif  // USE_AURA
 
 void ShellAppWindowClient::OpenDevToolsWindow(
     content::WebContents* web_contents,