Commit 9c61f630 authored by xiyuan@chromium.org's avatar xiyuan@chromium.org

chromeos: Fix StartupBrowserCreatorImpl::Launch crash.

Previous fix r144228 does not fix the crash. It seems the crash is more likely
to be caused by user closing the app window in a nested message that runs by
synchronous session restore. Add an WebContentsCloseObserver to monitor the app
contents and skip focusing logic if it is closed.

BUG=134478
TEST=Verify the crash in 134478 no longer happens.


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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@146027 0039d316-1c4b-4281-b951-d872f2087c98
parent 7ee8e5ba
...@@ -72,6 +72,8 @@ ...@@ -72,6 +72,8 @@
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/browser_distribution.h"
#include "content/public/browser/child_process_security_policy.h" #include "content/public/browser/child_process_security_policy.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_view.h" #include "content/public/browser/web_contents_view.h"
#include "grit/locale_settings.h" #include "grit/locale_settings.h"
...@@ -225,6 +227,37 @@ void RecordAppLaunches(Profile* profile, ...@@ -225,6 +227,37 @@ void RecordAppLaunches(Profile* profile,
} }
} }
class WebContentsCloseObserver : public content::NotificationObserver {
public:
WebContentsCloseObserver() : contents_(NULL) {}
virtual ~WebContentsCloseObserver() {}
void SetContents(content::WebContents* contents) {
DCHECK(!contents_);
contents_ = contents;
registrar_.Add(this,
content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
content::Source<content::WebContents>(contents_));
}
content::WebContents* contents() { return contents_; }
private:
// content::NotificationObserver overrides:
virtual void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE {
DCHECK_EQ(type, content::NOTIFICATION_WEB_CONTENTS_DESTROYED);
contents_ = NULL;
}
content::WebContents* contents_;
content::NotificationRegistrar registrar_;
DISALLOW_COPY_AND_ASSIGN(WebContentsCloseObserver);
};
} // namespace } // namespace
StartupBrowserCreatorImpl::StartupBrowserCreatorImpl( StartupBrowserCreatorImpl::StartupBrowserCreatorImpl(
...@@ -308,6 +341,13 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile, ...@@ -308,6 +341,13 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
// affecting browser startup have been detected. // affecting browser startup have been detected.
CheckPreferencesBackup(profile); CheckPreferencesBackup(profile);
// Watch for |app_contents| closing since ProcessLaunchURLs might run a
// synchronous session restore which has a nested message loop and could
// close |app_contents|.
WebContentsCloseObserver app_contents_observer;
if (browser_defaults::kAppRestoreSession && app_contents)
app_contents_observer.SetContents(app_contents);
ProcessLaunchURLs(process_startup, urls_to_open); ProcessLaunchURLs(process_startup, urls_to_open);
// If this is an app launch, but we didn't open an app window, it may // If this is an app launch, but we didn't open an app window, it may
...@@ -315,8 +355,8 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile, ...@@ -315,8 +355,8 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
OpenApplicationTab(profile); OpenApplicationTab(profile);
// In case of app mode + session restore we want to focus that app. // In case of app mode + session restore we want to focus that app.
if (browser_defaults::kAppRestoreSession && app_contents) if (app_contents_observer.contents())
app_contents->GetView()->SetInitialFocus(); app_contents_observer.contents()->GetView()->SetInitialFocus();
if (process_startup) { if (process_startup) {
if (browser_defaults::kOSSupportsOtherBrowsers && if (browser_defaults::kOSSupportsOtherBrowsers &&
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment