Skip to content

Commit

Permalink
Restart apps that don't listen to onRestarted() by sending them onLau…
Browse files Browse the repository at this point in the history
…nched().

We only do this if the app had windows opened the last time it was running,
otherwise apps that were running but had no windows may restart by opening
windows, which manifests as apps coming back on a restart after the user
already closed them.

BUG=230667,167740,162057

Review URL: https://chromiumcodereview.appspot.com/14878008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@199406 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
koz@chromium.org committed May 10, 2013
1 parent 0732b45 commit 95ef3fe
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 9 deletions.
45 changes: 45 additions & 0 deletions apps/app_restore_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h"
#include "chrome/browser/extensions/event_router.h"
#include "chrome/browser/extensions/extension_host.h"
#include "chrome/browser/extensions/extension_prefs.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
#include "chrome/browser/extensions/platform_app_launcher.h"
#include "chrome/browser/ui/extensions/shell_window.h"
#include "chrome/common/chrome_notification_types.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_set.h"
Expand Down Expand Up @@ -55,6 +57,9 @@ AppRestoreService::AppRestoreService(Profile* profile)
registrar_.Add(
this, chrome::NOTIFICATION_APP_TERMINATING,
content::NotificationService::AllSources());
extensions::ShellWindowRegistry* shell_window_registry =
extensions::ShellWindowRegistry::Get(profile);
shell_window_registry->AddObserver(this);
}

void AppRestoreService::HandleStartup(bool should_restore_apps) {
Expand Down Expand Up @@ -102,11 +107,34 @@ void AppRestoreService::Observe(int type,
// Stop listening to NOTIFICATION_EXTENSION_HOST_DESTROYED in particular
// as all extension hosts will be destroyed as a result of shutdown.
registrar_.RemoveAll();
// Stop listening to the ShellWindowRegistry for window closes, because
// all windows will be closed as a result of shutdown.
extensions::ShellWindowRegistry* shell_window_registry =
extensions::ShellWindowRegistry::Get(profile_);
shell_window_registry->RemoveObserver(this);
break;
}
}
}

void AppRestoreService::OnShellWindowAdded(ShellWindow* shell_window) {
RecordIfAppHasWindows(shell_window->extension()->id());
}

void AppRestoreService::OnShellWindowIconChanged(ShellWindow* shell_window) {
}

void AppRestoreService::OnShellWindowRemoved(ShellWindow* shell_window) {
// A shell window may be getting removed because its extension is uninstalled.
if (shell_window->extension())
RecordIfAppHasWindows(shell_window->extension()->id());
}

void AppRestoreService::Shutdown() {
extensions::ShellWindowRegistry* shell_window_registry =
extensions::ShellWindowRegistry::Get(profile_);
shell_window_registry->RemoveObserver(this);
}

void AppRestoreService::RecordAppStart(const std::string& extension_id) {
ExtensionPrefs* extension_prefs =
Expand All @@ -122,6 +150,23 @@ void AppRestoreService::RecordAppStop(const std::string& extension_id) {
extension_prefs, extension_id);
}

void AppRestoreService::RecordIfAppHasWindows(
const std::string& id) {
ExtensionService* extension_service =
ExtensionSystem::Get(profile_)->extension_service();
ExtensionPrefs* extension_prefs = extension_service->extension_prefs();

// If the extension isn't running then we will already have recorded whether
// it had windows or not.
if (!extension_prefs->IsExtensionRunning(id))
return;

extensions::ShellWindowRegistry* shell_window_registry =
extensions::ShellWindowRegistry::Get(profile_);
bool has_windows = !shell_window_registry->GetShellWindowsForApp(id).empty();
extension_prefs->SetHasWindows(id, has_windows);
}

void AppRestoreService::RestoreApp(
const Extension* extension,
const std::vector<SavedFileEntry>& file_entries) {
Expand Down
14 changes: 13 additions & 1 deletion apps/app_restore_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <string>
#include <vector>

#include "chrome/browser/extensions/shell_window_registry.h"
#include "chrome/browser/profiles/profile_keyed_service.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
Expand All @@ -29,7 +30,8 @@ namespace apps {

// Tracks what apps need to be restarted when the browser restarts.
class AppRestoreService : public ProfileKeyedService,
public content::NotificationObserver {
public content::NotificationObserver,
public extensions::ShellWindowRegistry::Observer {
public:
// Returns true if apps should be restored on the current platform, given
// whether this new browser process launched due to a restart.
Expand All @@ -47,8 +49,18 @@ class AppRestoreService : public ProfileKeyedService,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;

// extensions::ShellWindowRegistry::Observer.
virtual void OnShellWindowAdded(ShellWindow* shell_window) OVERRIDE;
virtual void OnShellWindowIconChanged(ShellWindow* shell_window) OVERRIDE;
virtual void OnShellWindowRemoved(ShellWindow* shell_window) OVERRIDE;

// ProfileKeyedService.
virtual void Shutdown() OVERRIDE;

void RecordAppStart(const std::string& extension_id);
void RecordAppStop(const std::string& extension_id);
void RecordIfAppHasWindows(const std::string& id);

void RestoreApp(
const extensions::Extension* extension,
const std::vector<SavedFileEntry>& file_entries);
Expand Down
2 changes: 2 additions & 0 deletions apps/app_restore_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "apps/app_restore_service_factory.h"

#include "apps/app_restore_service.h"
#include "chrome/browser/extensions/shell_window_registry.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_dependency_manager.h"

Expand All @@ -30,6 +31,7 @@ AppRestoreServiceFactory* AppRestoreServiceFactory::GetInstance() {
AppRestoreServiceFactory::AppRestoreServiceFactory()
: ProfileKeyedServiceFactory("AppRestoreService",
ProfileDependencyManager::GetInstance()) {
DependsOn(extensions::ShellWindowRegistry::Factory::GetInstance());
}

AppRestoreServiceFactory::~AppRestoreServiceFactory() {
Expand Down
18 changes: 18 additions & 0 deletions chrome/browser/extensions/extension_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ namespace {
// Whether this extension was running when chrome last shutdown.
const char kPrefRunning[] = "running";

// Whether this extension had windows when it was last running.
const char kHasWindows[] = "has_windows";

// Where an extension was installed from. (see Manifest::Location)
const char kPrefLocation[] = "location";

Expand Down Expand Up @@ -1146,6 +1149,21 @@ bool ExtensionPrefs::IsExtensionRunning(const std::string& extension_id) {
return running;
}

void ExtensionPrefs::SetHasWindows(const std::string& extension_id,
bool has_windows) {
Value* value = Value::CreateBooleanValue(has_windows);
UpdateExtensionPref(extension_id, kHasWindows, value);
}

bool ExtensionPrefs::HasWindows(const std::string& extension_id) {
const DictionaryValue* extension = GetExtensionPref(extension_id);
if (!extension)
return false;
bool has_windows = false;
extension->GetBoolean(kHasWindows, &has_windows);
return has_windows;
}

bool ExtensionPrefs::IsIncognitoEnabled(const std::string& extension_id) {
return ReadPrefAsBooleanAndReturn(extension_id, kPrefIncognitoEnabled);
}
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/extensions/extension_prefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,12 @@ class ExtensionPrefs : public ContentSettingsStore::Observer,
// restart apps across browser restarts.
bool IsExtensionRunning(const std::string& extension_id);

// Set/Get whether or not the extension has windows associated with it. Used
// to force a launch of apps that don't handle onRestarted() on a restart. We
// can only safely do that if the app had windows when it was last running.
void SetHasWindows(const std::string& extension_id, bool has_windows);
bool HasWindows(const std::string& extension_id);

// Returns true if the user enabled this extension to be loaded in incognito
// mode.
bool IsIncognitoEnabled(const std::string& extension_id);
Expand Down
26 changes: 23 additions & 3 deletions chrome/browser/extensions/platform_app_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "chrome/browser/extensions/api/app_runtime/app_runtime_api.h"
#include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h"
#include "chrome/browser/extensions/api/file_system/file_system_api.h"
#include "chrome/browser/extensions/event_names.h"
#include "chrome/browser/extensions/event_router.h"
#include "chrome/browser/extensions/extension_host.h"
#include "chrome/browser/extensions/extension_prefs.h"
#include "chrome/browser/extensions/extension_process_manager.h"
Expand Down Expand Up @@ -421,9 +423,27 @@ void RestartPlatformAppWithFileEntries(
Profile* profile,
const Extension* extension,
const std::vector<SavedFileEntry>& file_entries) {
scoped_refptr<SavedFileEntryLauncher> launcher = new SavedFileEntryLauncher(
profile, extension, file_entries);
launcher->Launch();
extensions::EventRouter* event_router =
ExtensionSystem::Get(profile)->event_router();
bool listening_to_restart = event_router->
ExtensionHasEventListener(extension->id(), event_names::kOnRestarted);

if (listening_to_restart) {
scoped_refptr<SavedFileEntryLauncher> launcher = new SavedFileEntryLauncher(
profile, extension, file_entries);
launcher->Launch();
return;
}

ExtensionPrefs* extension_prefs = ExtensionSystem::Get(profile)->
extension_service()->extension_prefs();
bool had_windows = extension_prefs->HasWindows(extension->id());
extension_prefs->SetHasWindows(extension->id(), false);
bool listening_to_launch = event_router->
ExtensionHasEventListener(extension->id(), event_names::kOnLaunched);

if (listening_to_launch && had_windows)
LaunchPlatformAppWithNoData(profile, extension);
}

} // namespace extensions
8 changes: 4 additions & 4 deletions chrome/browser/extensions/shell_window_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ class ShellWindowRegistry : public ProfileKeyedService {
// ShellWindow::WindowType, or 0 for any window type.
static bool IsShellWindowRegisteredInAnyProfile(int window_type_mask);

protected:
void OnDevToolsStateChanged(content::DevToolsAgentHost*, bool attached);

private:
class Factory : public ProfileKeyedServiceFactory {
public:
static ShellWindowRegistry* GetForProfile(Profile* profile, bool create);
Expand All @@ -122,6 +118,10 @@ class ShellWindowRegistry : public ProfileKeyedService {
content::BrowserContext* context) const OVERRIDE;
};

protected:
void OnDevToolsStateChanged(content::DevToolsAgentHost*, bool attached);

private:
Profile* profile_;
ShellWindowSet shell_windows_;
InspectedWindowSet inspected_windows_;
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/ui/extensions/shell_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,10 @@ void ShellWindow::Observe(int type,
const extensions::Extension* unloaded_extension =
content::Details<extensions::UnloadedExtensionInfo>(
details)->extension;
if (extension_ == unloaded_extension)
if (extension_ == unloaded_extension) {
native_app_window_->Close();
extension_ = NULL;
}
break;
}
case chrome::NOTIFICATION_APP_TERMINATING:
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/extensions/shell_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ class ShellWindow : public content::NotificationObserver,

Profile* profile_; // weak pointer - owned by ProfileManager.
// weak pointer - owned by ExtensionService.
// This gets set to NULL when the extension is uninstalled.
const extensions::Extension* extension_;

// Identifier that is used when saving and restoring geometry for this
Expand Down

0 comments on commit 95ef3fe

Please sign in to comment.