Revert 112770 - Prevent a crash in the first run search engine selector.

It is possible to try and close the window now before the TemplateURLService is loaded. In this case we will not have a fallback search engine to select by default. So we will just reject attempts to close the window in this circumstance until it is loaded and a fallback choice has been set. It should be an edge case.

http://crbug.com/106078
TEST=unit test
Review URL: http://codereview.chromium.org/8771018

TBR=ben@chromium.org
Review URL: http://codereview.chromium.org/8790001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112804 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 3 insertions, 78 deletions

diff --git a/chrome/browser/ui/views/first_run_search_engine_view.cc b/chrome/browser/ui/views/first_run_search_engine_view.cc
index 9884b05..1f1e13f 100644
--- a/chrome/browser/ui/views/first_run_search_engine_view.cc
+++ b/chrome/browser/ui/views/first_run_search_engine_view.cc
@@ -169,13 +169,6 @@ string16 FirstRunSearchEngineView::GetWindowTitle() const {
   return l10n_util::GetStringUTF16(IDS_FIRSTRUN_DLG_TITLE);
 }
 
-bool FirstRunSearchEngineView::CanClose() {
-  // We need a valid search engine to set as default, so if the user tries to
-  // close the window before the template URL service is loaded, we must prevent
-  // this from happening.
-  return fallback_choice_ != NULL;
-}
-
 void FirstRunSearchEngineView::WindowClosing() {
   // If the window is closed by clicking the close button, we default to the
   // engine in the first slot.
@@ -484,7 +477,7 @@ void FirstRunSearchEngineView::AddSearchEnginesIfPossible() {
 
 void FirstRunSearchEngineView::ChooseSearchEngine(SearchEngineChoice* choice) {
   user_chosen_engine_ = true;
-  DCHECK(choice && template_url_service_);
+  DCHECK(template_url_service_);
   template_url_service_->SetSearchEngineDialogSlot(choice->slot());
   const TemplateURL* default_search = choice->GetSearchEngine();
   if (default_search)
diff --git a/chrome/browser/ui/views/first_run_search_engine_view.h b/chrome/browser/ui/views/first_run_search_engine_view.h
index 66bcaee..28ef439 100644
--- a/chrome/browser/ui/views/first_run_search_engine_view.h
+++ b/chrome/browser/ui/views/first_run_search_engine_view.h
@@ -87,7 +87,6 @@ class FirstRunSearchEngineView : public views::WidgetDelegateView,
   // Overridden from views::WidgetDelegateView:
   virtual string16 GetWindowTitle() const OVERRIDE;
   virtual views::View* GetContentsView() OVERRIDE { return this; }
-  virtual bool CanClose() OVERRIDE;
   virtual void WindowClosing() OVERRIDE;
 
   // Overridden from views::ButtonListener:
diff --git a/chrome/browser/ui/views/first_run_search_engine_view_unittest.cc b/chrome/browser/ui/views/first_run_search_engine_view_unittest.cc
index 8d47890..f0e191f 100644
--- a/chrome/browser/ui/views/first_run_search_engine_view_unittest.cc
+++ b/chrome/browser/ui/views/first_run_search_engine_view_unittest.cc
@@ -7,12 +7,7 @@
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
-#include "chrome/common/chrome_notification_types.h"
 #include "chrome/test/base/testing_profile.h"
-#include "chrome/test/base/ui_test_utils.h"
-#include "content/browser/browser_process_sub_thread.h"
-#include "content/public/browser/notification_service.h"
-#include "content/test/test_browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/views/focus/accelerator_handler.h"
 #include "ui/views/test/views_test_base.h"
@@ -54,56 +49,3 @@ TEST_F(FirstRunSearchEngineViewTest, ClosingSelectsFirstEngine) {
   ASSERT_TRUE(!template_urls.empty());
   EXPECT_EQ(template_urls.front(), service->GetDefaultSearchProvider());
 }
-
-TEST_F(FirstRunSearchEngineViewTest, ClosingBeforeServiceLoadedAbortsClose) {
-  // This ensures the current thread is named the UI thread, so code that checks
-  // that this is the UI thread doesn't assert.
-  content::TestBrowserThread ui_thread(content::BrowserThread::UI,
-                                       message_loop());
-  content::BrowserProcessSubThread db_thread(content::BrowserThread::DB);
-  db_thread.StartWithOptions(base::Thread::Options());
-
-  TestingProfile profile;
-  // We need to initialize the web database before accessing the template url
-  // service otherwise the template url service will init itself synchronously
-  // and appear to be loaded.
-  profile.CreateWebDataService(false);
-  profile.CreateTemplateURLService();
-
-  // Instead of giving the template url service a chance to load, try and close
-  // the window immediately.
-  FirstRunSearchEngineView* contents =
-      new FirstRunSearchEngineView(&profile, false);
-  contents->set_quit_on_closing(false);
-  views::Widget* window = views::Widget::CreateWindow(contents);
-  window->Show();
-  EXPECT_TRUE(window->IsVisible());
-  window->Close();
-  // The window wouldn't actually be closed until a return to the message loop,
-  // which we don't want to spin here because the window shouldn't have closed
-  // in the correct case. The window is however actually hidden immediately when
-  // the window is allowed to close, so we can test for visibility to make sure
-  // it hasn't.
-  EXPECT_TRUE(window->IsVisible());
-
-  // Now let the template url service a chance to load.
-  ui_test_utils::WindowedNotificationObserver service_load_observer(
-      chrome::NOTIFICATION_TEMPLATE_URL_SERVICE_LOADED,
-      content::NotificationService::AllSources());
-  service_load_observer.Wait();
-
-  // .. and try and close the window. It should be allowed to now.
-  window->Close();
-  EXPECT_FALSE(window->IsVisible());
-
-  // Allow the window to actually close.
-  RunPendingMessages();
-
-  // Verify goodness. Because we actually went to the trouble of starting the
-  // WebDB, we will have real data in the model, so we can verify a choice was
-  // made without having to seed the model with dummy data.
-  TemplateURLService* service =
-      TemplateURLServiceFactory::GetForProfile(&profile);
-  ASSERT_TRUE(service != NULL);
-  EXPECT_TRUE(service->GetDefaultSearchProvider() != NULL);
-}
diff --git a/ui/views/test/views_test_base.h b/ui/views/test/views_test_base.h
index eed6889..51a9df6 100644
--- a/ui/views/test/views_test_base.h
+++ b/ui/views/test/views_test_base.h
@@ -35,8 +35,6 @@ class ViewsTestBase : public testing::Test {
     views_delegate_.reset(views_delegate);
   }
 
-  MessageLoop* message_loop() { return &message_loop_; }
-
  private:
   MessageLoopForUI message_loop_;
   scoped_ptr<TestViewsDelegate> views_delegate_;
diff --git a/ui/views/widget/widget.cc b/ui/views/widget/widget.cc
index deeaa38..472e284 100644
--- a/ui/views/widget/widget.cc
+++ b/ui/views/widget/widget.cc
@@ -466,8 +466,8 @@ void Widget::Close() {
     return;
   }
 
-  bool can_close = widget_delegate_->CanClose();
-  if (non_client_view_ && can_close)
+  bool can_close = true;
+  if (non_client_view_)
     can_close = non_client_view_->CanClose();
   if (can_close) {
     SaveWindowPlacement();
diff --git a/ui/views/widget/widget_delegate.cc b/ui/views/widget/widget_delegate.cc
index dbbd438..6456c61 100644
--- a/ui/views/widget/widget_delegate.cc
+++ b/ui/views/widget/widget_delegate.cc
@@ -128,10 +128,6 @@ bool WidgetDelegate::ShouldRestoreWindowSize() const {
   return true;
 }
 
-bool WidgetDelegate::CanClose() {
-  return true;
-}
-
 View* WidgetDelegate::GetContentsView() {
   if (!default_contents_view_)
     default_contents_view_ = new View;
diff --git a/ui/views/widget/widget_delegate.h b/ui/views/widget/widget_delegate.h
index ea48930..58a2c72 100644
--- a/ui/views/widget/widget_delegate.h
+++ b/ui/views/widget/widget_delegate.h
@@ -117,9 +117,6 @@ class VIEWS_EXPORT WidgetDelegate {
   // Default is true.
   virtual bool ShouldRestoreWindowSize() const;
 
-  // Return false to indicate that the window can't be closed. Defaults to true.
-  virtual bool CanClose();
-
   // Called when the window closes. The delegate MUST NOT delete itself during
   // this call, since it can be called afterwards. See DeleteDelegate().
   virtual void WindowClosing() {}