From 8a7ffed9adaa1fd9c954e8ec6f8823d1705ec6e9 Mon Sep 17 00:00:00 2001 From: "benwells@chromium.org" Date: Wed, 17 Jul 2013 06:21:19 +0000 Subject: [PATCH] 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 --- .../extensions => apps}/app_window_contents.cc | 14 ++++---------- .../extensions => apps}/app_window_contents.h | 14 +++++++++----- apps/apps.gypi | 2 ++ apps/shell_window.cc | 15 ++------------- apps/shell_window.h | 8 -------- chrome/browser/extensions/DEPS | 1 + .../extensions/api/app_window/app_window_api.cc | 10 ++++++---- chrome/browser/extensions/api/tabs/tabs_api.cc | 3 --- .../extensions/platform_app_browsertest_util.cc | 17 ++++++++++------- .../ui/apps/chrome_shell_window_delegate.cc | 7 +++++++ chrome/chrome_browser_extensions.gypi | 2 -- 11 files changed, 41 insertions(+), 52 deletions(-) rename {chrome/browser/extensions => apps}/app_window_contents.cc (94%) rename {chrome/browser/extensions => apps}/app_window_contents.h (89%) diff --git a/chrome/browser/extensions/app_window_contents.cc b/apps/app_window_contents.cc similarity index 94% rename from chrome/browser/extensions/app_window_contents.cc rename to apps/app_window_contents.cc index f1f72ec71c807b..bcefa3ef8cc842 100644 --- a/chrome/browser/extensions/app_window_contents.cc +++ b/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/chrome/browser/extensions/app_window_contents.h b/apps/app_window_contents.h similarity index 89% rename from chrome/browser/extensions/app_window_contents.h rename to apps/app_window_contents.h index bb377875d378f7..80b95de3d6793e 100644 --- a/chrome/browser/extensions/app_window_contents.h +++ b/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& 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 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 index 83e1bbdb5845c7..92a74a76849fa2 100644 --- 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 index 23993dd9a327e6..3fd2314f28fa1d 100644 --- 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 index f283c8008c2085..5e449df2dcf572 100644 --- 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 index 05df41bfddf265..890d2bad5754b1 100644 --- 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 index 9f9c3546b057bc..96306e32a1404f 100644 --- 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 index 9b097a6de63bca..32134b31adda09 100644 --- 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 index 2d1cdd0fd9b89c..a2634c70855a29 100644 --- 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 index 2ed133330948cc..80b810ffc3bd6b 100644 --- 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 index d113225197b9e6..6c7c369ee38d43 100644 --- 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',