Move AppWindowContents to apps component

This change also: - moves some chrome specific logic from AppWindowContents to the chrome shell window delegate - gets rid of the app-only ShellWindow::Create so apps and panels create shell windows in the same way - moves registering with the registry to the constructor, from constructor call sites. BUG=159366 Review URL: https://chromiumcodereview.appspot.com/19272010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211974 0039d316-1c4b-4281-b951-d872f2087c98
qweri0p · Jul 17, 2013 · 8a7ffed · 8a7ffed
1 parent ae82919
commit 8a7ffed
Show file tree

Hide file tree

Showing 11 changed files with 41 additions and 52 deletions.
diff --git a/...browser/extensions/app_window_contents.cc → apps/app_window_contents.cc b/...browser/extensions/app_window_contents.cc → apps/app_window_contents.cc
@@ -2,11 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/extensions/app_window_contents.h"
+#include "apps/app_window_contents.h"
 
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/printing/print_preview_message_handler.h"
-#include "chrome/browser/printing/print_view_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/extensions/native_app_window.h"
 #include "chrome/common/extensions/api/app_window.h"
@@ -21,7 +19,7 @@
 
 namespace app_window = extensions::api::app_window;
 
-using apps::ShellWindow;
+namespace apps {
 
 AppWindowContents::AppWindowContents(ShellWindow* host)
     : host_(host) {
@@ -44,12 +42,6 @@ void AppWindowContents::Initialize(Profile* profile, const GURL& url) {
   web_contents_->GetMutableRendererPrefs()->
       browser_handles_all_top_level_requests = true;
   web_contents_->GetRenderViewHost()->SyncRendererPrefs();
-
-#if defined(ENABLE_PRINTING)
-  printing::PrintPreviewMessageHandler::CreateForWebContents(
-      web_contents_.get());
-  printing::PrintViewManager::CreateForWebContents(web_contents_.get());
-#endif
 }
 
 void AppWindowContents::LoadContents(int32 creator_process_id) {
@@ -181,3 +173,5 @@ void AppWindowContents::SuspendRenderViewHost(
                  base::Unretained(content::ResourceDispatcherHost::Get()),
                  rvh->GetProcess()->GetID(), rvh->GetRoutingID()));
 }
+
+}  // namespace apps
diff --git a/.../browser/extensions/app_window_contents.h → apps/app_window_contents.h b/.../browser/extensions/app_window_contents.h → apps/app_window_contents.h
@@ -24,18 +24,20 @@ namespace extensions {
 struct DraggableRegion;
 }
 
-// apps::ShellWindowContents class specific to app windows. It maintains a
+namespace apps {
+
+// ShellWindowContents class specific to app windows. It maintains a
 // WebContents instance and observes it for the purpose of passing
 // messages to the extensions system.
-class AppWindowContents : public apps::ShellWindowContents,
+class AppWindowContents : public ShellWindowContents,
                           public content::NotificationObserver,
                           public content::WebContentsObserver,
                           public ExtensionFunctionDispatcher::Delegate {
  public:
-  explicit AppWindowContents(apps::ShellWindow* host);
+  explicit AppWindowContents(ShellWindow* host);
   virtual ~AppWindowContents();
 
-  // apps::ShellWindowContents
+  // ShellWindowContents
   virtual void Initialize(Profile* profile, const GURL& url) OVERRIDE;
   virtual void LoadContents(int32 creator_process_id) OVERRIDE;
   virtual void NativeWindowChanged(NativeAppWindow* native_app_window) OVERRIDE;
@@ -61,7 +63,7 @@ class AppWindowContents : public apps::ShellWindowContents,
       const std::vector<extensions::DraggableRegion>& regions);
   void SuspendRenderViewHost(content::RenderViewHost* rvh);
 
-  apps::ShellWindow* host_;  // This class is owned by |host_|
+  ShellWindow* host_;  // This class is owned by |host_|
   GURL url_;
   content::NotificationRegistrar registrar_;
   scoped_ptr<content::WebContents> web_contents_;
@@ -70,4 +72,6 @@ class AppWindowContents : public apps::ShellWindowContents,
   DISALLOW_COPY_AND_ASSIGN(AppWindowContents);
 };
 
+}  // namespace apps
+
 #endif  // CHROME_BROWSER_EXTENSIONS_APP_WINDOW_CONTENTS_H_
diff --git a/apps/apps.gypi b/apps/apps.gypi
@@ -47,6 +47,8 @@
         'app_shim/chrome_main_app_mode_mac.mm',
         'app_shim/extension_app_shim_handler_mac.cc',
         'app_shim/extension_app_shim_handler_mac.h',
+        'app_window_contents.cc',
+        'app_window_contents.h',
         'field_trial_names.cc',
         'field_trial_names.h',
         'metrics_names.h',

diff --git a/apps/shell_window.cc b/apps/shell_window.cc
@@ -9,7 +9,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/extensions/app_window_contents.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/image_loader.h"
@@ -68,18 +67,6 @@ ShellWindow::CreateParams::~CreateParams() {}
 
 ShellWindow::Delegate::~Delegate() {}
 
-ShellWindow* ShellWindow::Create(Profile* profile,
-                                 Delegate* delegate,
-                                 const extensions::Extension* extension,
-                                 const GURL& url,
-                                 const CreateParams& params) {
-  // This object will delete itself when the window is closed.
-  ShellWindow* window = new ShellWindow(profile, delegate, extension);
-  window->Init(url, new AppWindowContents(window), params);
-  extensions::ShellWindowRegistry::Get(profile)->AddShellWindow(window);
-  return window;
-}
-
 ShellWindow::ShellWindow(Profile* profile,
                          Delegate* delegate,
                          const extensions::Extension* extension)
@@ -211,6 +198,8 @@ void ShellWindow::Init(const GURL& url,
   chrome::StartKeepAlive();
 
   UpdateExtensionAppIcon();
+
+  extensions::ShellWindowRegistry::Get(profile_)->AddShellWindow(this);
 }
 
 ShellWindow::~ShellWindow() {

diff --git a/apps/shell_window.h b/apps/shell_window.h
@@ -160,14 +160,6 @@ class ShellWindow : public content::NotificationObserver,
     virtual bool IsWebContentsVisible(content::WebContents* web_contents) = 0;
   };
 
-  // Helper function for creating and intiailizing a v2 app window.
-  // The created shell window will take ownership of |delegate|.
-  static ShellWindow* Create(Profile* profile,
-                             Delegate* delegate,
-                             const extensions::Extension* extension,
-                             const GURL& url,
-                             const CreateParams& params);
-
   // Convert draggable regions in raw format to SkRegion format. Caller is
   // responsible for deleting the returned SkRegion instance.
   static SkRegion* RawDraggableRegionsToSkRegion(

diff --git a/chrome/browser/extensions/DEPS b/chrome/browser/extensions/DEPS
@@ -4,6 +4,7 @@ include_rules = [
   # chrome/browser/extensions, the restriction of not being able
   # to depend on apps will be lifted.
   "-apps",
+  "+apps/app_window_contents.h",
   "+apps/shell_window.h",
 
   # TODO(tfarina): Remove all these. crbug.com/125846.

diff --git a/chrome/browser/extensions/api/app_window/app_window_api.cc b/chrome/browser/extensions/api/app_window/app_window_api.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/api/app_window/app_window_api.h"
 
+#include "apps/app_window_contents.h"
 #include "apps/shell_window.h"
 #include "base/command_line.h"
 #include "base/time/time.h"
@@ -292,12 +293,13 @@ bool AppWindowCreateFunction::RunImpl() {
   if (force_maximize)
     create_params.state = ui::SHOW_STATE_MAXIMIZED;
 
-  ShellWindow* shell_window = ShellWindow::Create(
+  ShellWindow* shell_window = new ShellWindow(
       profile(),
       new chrome::ChromeShellWindowDelegate(),
-      GetExtension(),
-      url,
-      create_params);
+      GetExtension());
+  shell_window->Init(url,
+                     new apps::AppWindowContents(shell_window),
+                     create_params);
 
   if (chrome::IsRunningInForcedAppMode())
     shell_window->Fullscreen();

diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -594,9 +594,6 @@ bool WindowsCreateFunction::RunImpl() {
       shell_window->Init(urls[0], ash_panel_contents, create_params);
       SetResult(ash_panel_contents->GetExtensionWindowController()->
                 CreateWindowValueWithTabs(GetExtension()));
-      // Add the panel to the shell window registry so that it shows up in
-      // the launcher and as an active render process.
-      ShellWindowRegistry::Get(window_profile)->AddShellWindow(shell_window);
       return true;
     }
 #else

diff --git a/chrome/browser/extensions/platform_app_browsertest_util.cc b/chrome/browser/extensions/platform_app_browsertest_util.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/platform_app_browsertest_util.h"
 
+#include "apps/app_window_contents.h"
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/tabs/tabs_api.h"
@@ -146,17 +147,19 @@ void PlatformAppBrowserTest::SetCommandLineArg(const std::string& test_file) {
 
 ShellWindow* PlatformAppBrowserTest::CreateShellWindow(
     const Extension* extension) {
-  ShellWindow::CreateParams params;
-  return ShellWindow::Create(
-      browser()->profile(), new chrome::ChromeShellWindowDelegate(),
-      extension, GURL(std::string()), params);
+  return CreateShellWindowFromParams(extension, ShellWindow::CreateParams());
 }
 
 ShellWindow* PlatformAppBrowserTest::CreateShellWindowFromParams(
     const Extension* extension, const ShellWindow::CreateParams& params) {
-  return ShellWindow::Create(
-      browser()->profile(), new chrome::ChromeShellWindowDelegate(),
-      extension, GURL(std::string()), params);
+  ShellWindow* window = new ShellWindow(
+      browser()->profile(),
+      new chrome::ChromeShellWindowDelegate(),
+      extension);
+  window->Init(GURL(std::string()),
+               new apps::AppWindowContents(window),
+               params);
+  return window;
 }
 
 void PlatformAppBrowserTest::CloseShellWindow(ShellWindow* window) {

diff --git a/chrome/browser/ui/apps/chrome_shell_window_delegate.cc b/chrome/browser/ui/apps/chrome_shell_window_delegate.cc
@@ -9,6 +9,8 @@
 #include "chrome/browser/file_select_helper.h"
 #include "chrome/browser/media/media_capture_devices_dispatcher.h"
 #include "chrome/browser/platform_util.h"
+#include "chrome/browser/printing/print_preview_message_handler.h"
+#include "chrome/browser/printing/print_view_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -56,6 +58,11 @@ void ChromeShellWindowDelegate::DisableExternalOpenForTesting() {
 void ChromeShellWindowDelegate::InitWebContents(
     content::WebContents* web_contents) {
   FaviconTabHelper::CreateForWebContents(web_contents);
+
+#if defined(ENABLE_PRINTING)
+  printing::PrintPreviewMessageHandler::CreateForWebContents(web_contents);
+  printing::PrintViewManager::CreateForWebContents(web_contents);
+#endif
 }
 
 content::WebContents* ChromeShellWindowDelegate::OpenURLFromTab(

diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi
@@ -521,8 +521,6 @@
         'browser/extensions/app_sync_bundle.h',
         'browser/extensions/app_sync_data.cc',
         'browser/extensions/app_sync_data.h',
-        'browser/extensions/app_window_contents.cc',
-        'browser/extensions/app_window_contents.h',
         'browser/extensions/blacklist.cc',
         'browser/extensions/blacklist.h',
         'browser/extensions/browser_action_test_util.h',