From bc4ade2129b3d7464b36bc480e607b92ff3eb9ec Mon Sep 17 00:00:00 2001 From: "tfarina@chromium.org" Date: Sun, 27 May 2012 16:39:21 +0000 Subject: browser: Factor out TabContentsIterator into its own module. R=ben@chromium.org Review URL: https://chromiumcodereview.appspot.com/10453034 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@139216 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/chrome_browser_application_mac.mm | 2 +- chrome/browser/extensions/extension_tab_helper.cc | 2 +- chrome/browser/extensions/extension_web_ui.cc | 4 +- .../task_manager_resource_providers.cc | 1 + chrome/browser/ui/browser_finder.cc | 1 + chrome/browser/ui/browser_list.cc | 46 +--- chrome/browser/ui/browser_list.h | 75 +----- chrome/browser/ui/browser_list_unittest.cc | 273 -------------------- .../browser/ui/cocoa/hung_renderer_controller.mm | 2 +- chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc | 2 +- .../ui/tab_contents/tab_contents_iterator.cc | 59 +++++ .../ui/tab_contents/tab_contents_iterator.h | 85 +++++++ .../tab_contents/tab_contents_iterator_unittest.cc | 274 +++++++++++++++++++++ chrome/browser/ui/views/hung_renderer_view.cc | 2 +- chrome/browser/ui/webui/inspect_ui.cc | 9 +- chrome/chrome_browser.gypi | 2 + chrome/chrome_tests.gypi | 4 +- 17 files changed, 437 insertions(+), 406 deletions(-) delete mode 100644 chrome/browser/ui/browser_list_unittest.cc create mode 100644 chrome/browser/ui/tab_contents/tab_contents_iterator.cc create mode 100644 chrome/browser/ui/tab_contents/tab_contents_iterator.h create mode 100644 chrome/browser/ui/tab_contents/tab_contents_iterator_unittest.cc diff --git a/chrome/browser/chrome_browser_application_mac.mm b/chrome/browser/chrome_browser_application_mac.mm index 1cd2e5d..f4de295 100644 --- a/chrome/browser/chrome_browser_application_mac.mm +++ b/chrome/browser/chrome_browser_application_mac.mm @@ -12,7 +12,7 @@ #import "base/metrics/histogram.h" #import "base/sys_string_conversions.h" #import "chrome/browser/app_controller_mac.h" -#include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #import "chrome/common/mac/objc_method_swizzle.h" #import "chrome/common/mac/objc_zombie.h" diff --git a/chrome/browser/extensions/extension_tab_helper.cc b/chrome/browser/extensions/extension_tab_helper.cc index 35ce5be..9682749 100644 --- a/chrome/browser/extensions/extension_tab_helper.cc +++ b/chrome/browser/extensions/extension_tab_helper.cc @@ -12,7 +12,7 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sessions/restore_tab_helper.h" #include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/extensions/extension_action.h" diff --git a/chrome/browser/extensions/extension_web_ui.cc b/chrome/browser/extensions/extension_web_ui.cc index 429f5bb..3c19ed3 100644 --- a/chrome/browser/extensions/extension_web_ui.cc +++ b/chrome/browser/extensions/extension_web_ui.cc @@ -16,7 +16,7 @@ #include "chrome/browser/prefs/scoped_user_pref_update.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension.h" @@ -27,8 +27,8 @@ #include "content/public/browser/navigation_controller.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_ui.h" -#include "content/public/common/page_transition_types.h" #include "content/public/common/bindings_policy.h" +#include "content/public/common/page_transition_types.h" #include "net/base/file_stream.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/codec/png_codec.h" diff --git a/chrome/browser/task_manager/task_manager_resource_providers.cc b/chrome/browser/task_manager/task_manager_resource_providers.cc index 0b7568a..b5358c3 100644 --- a/chrome/browser/task_manager/task_manager_resource_providers.cc +++ b/chrome/browser/task_manager/task_manager_resource_providers.cc @@ -36,6 +36,7 @@ #include "chrome/browser/tab_contents/tab_util.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/browser/view_type_utils.h" #include "chrome/common/chrome_notification_types.h" diff --git a/chrome/browser/ui/browser_finder.cc b/chrome/browser/ui/browser_finder.cc index 16923b3..2d97402 100644 --- a/chrome/browser/ui/browser_finder.cc +++ b/chrome/browser/ui/browser_finder.cc @@ -7,6 +7,7 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_window.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "content/public/browser/navigation_controller.h" diff --git a/chrome/browser/ui/browser_list.cc b/chrome/browser/ui/browser_list.cc index 5bd9bfc..a8211e4 100644 --- a/chrome/browser/ui/browser_list.cc +++ b/chrome/browser/ui/browser_list.cc @@ -7,12 +7,12 @@ #include "base/command_line.h" #include "base/logging.h" #include "base/metrics/histogram.h" +#include "base/observer_list.h" #include "build/build_config.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_shutdown.h" #include "chrome/browser/lifetime/application_lifetime.h" #include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/printing/background_printing_manager.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_window.h" @@ -129,10 +129,6 @@ static ObserverList& observers() { return observer_vector; } -printing::BackgroundPrintingManager* GetBackgroundPrintingManager() { - return g_browser_process->background_printing_manager(); -} - } // namespace // static @@ -323,43 +319,3 @@ void BrowserList::RemoveBrowserFrom(Browser* browser, browser_list->erase(remove_browser); } -TabContentsIterator::TabContentsIterator() - : browser_iterator_(BrowserList::begin()), - web_view_index_(-1), - bg_printing_iterator_(GetBackgroundPrintingManager()->begin()), - cur_(NULL) { - Advance(); -} - -void TabContentsIterator::Advance() { - // The current WebContents should be valid unless we are at the beginning. - DCHECK(cur_ || (web_view_index_ == -1 && - browser_iterator_ == BrowserList::begin())) - << "Trying to advance past the end"; - - // Update cur_ to the next WebContents in the list. - while (browser_iterator_ != BrowserList::end()) { - if (++web_view_index_ >= (*browser_iterator_)->tab_count()) { - // Advance to the next Browser in the list. - ++browser_iterator_; - web_view_index_ = -1; - continue; - } - - TabContentsWrapper* next_tab = - (*browser_iterator_)->GetTabContentsWrapperAt(web_view_index_); - if (next_tab) { - cur_ = next_tab; - return; - } - } - // If no more WebContents from Browsers, check the BackgroundPrintingManager. - while (bg_printing_iterator_ != GetBackgroundPrintingManager()->end()) { - cur_ = *bg_printing_iterator_; - CHECK(cur_); - ++bg_printing_iterator_; - return; - } - // Reached the end - no more WebContents. - cur_ = NULL; -} diff --git a/chrome/browser/ui/browser_list.h b/chrome/browser/ui/browser_list.h index 7409221..ef5b6f9 100644 --- a/chrome/browser/ui/browser_list.h +++ b/chrome/browser/ui/browser_list.h @@ -6,18 +6,16 @@ #define CHROME_BROWSER_UI_BROWSER_LIST_H_ #pragma once -#include #include #include "base/gtest_prod_util.h" -#include "base/observer_list.h" #include "ui/gfx/native_widget_types.h" class Browser; class Profile; namespace content { - class WebContents; +class WebContents; } namespace browser { @@ -168,75 +166,4 @@ class BrowserList { static void RemoveBrowserFrom(Browser* browser, BrowserVector* browser_list); }; -class TabContentsWrapper; - -// Iterates through all web view hosts in all browser windows. Because the -// renderers act asynchronously, getting a host through this interface does -// not guarantee that the renderer is ready to go. Doing anything to affect -// browser windows or tabs while iterating may cause incorrect behavior. -// -// Example: -// for (TabContentsIterator iterator; !iterator.done(); ++iterator) { -// TabContentsWrapper* cur = *iterator; -// -or- -// iterator->operationOnTabContents(); -// ... -// } -class TabContentsIterator { - public: - TabContentsIterator(); - - // Returns true if we are past the last Browser. - bool done() const { - return cur_ == NULL; - } - - // Returns the Browser instance associated with the current - // TabContentsWrapper. Valid as long as !done() - Browser* browser() const { - if (browser_iterator_ != BrowserList::end()) - return *browser_iterator_; - return NULL; - } - - // Returns the current TabContentsWrapper, valid as long as !Done() - TabContentsWrapper* operator->() const { - return cur_; - } - TabContentsWrapper* operator*() const { - return cur_; - } - - // Incrementing operators, valid as long as !Done() - TabContentsWrapper* operator++() { // ++preincrement - Advance(); - return cur_; - } - TabContentsWrapper* operator++(int) { // postincrement++ - TabContentsWrapper* tmp = cur_; - Advance(); - return tmp; - } - - private: - // Loads the next host into Cur. This is designed so that for the initial - // call when browser_iterator_ points to the first browser and - // web_view_index_ is -1, it will fill the first host. - void Advance(); - - // iterator over all the Browser objects - BrowserList::const_iterator browser_iterator_; - - // tab index into the current Browser of the current web view - int web_view_index_; - - // iterator over the TabContentsWrappers doing background printing. - std::set::const_iterator bg_printing_iterator_; - - // Current TabContentsWrapper, or NULL if we're at the end of the list. This - // can be extracted given the browser iterator and index, but it's nice to - // cache this since the caller may access the current host many times. - TabContentsWrapper* cur_; -}; - #endif // CHROME_BROWSER_UI_BROWSER_LIST_H_ diff --git a/chrome/browser/ui/browser_list_unittest.cc b/chrome/browser/ui/browser_list_unittest.cc deleted file mode 100644 index 76d8411..0000000 --- a/chrome/browser/ui/browser_list_unittest.cc +++ /dev/null @@ -1,273 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/browser_process.h" -#include "chrome/browser/browser_shutdown.h" -#include "chrome/browser/lifetime/application_lifetime.h" -#include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/printing/background_printing_manager.h" -#include "chrome/browser/profiles/profile_manager.h" -#include "chrome/browser/ui/browser_list.h" -#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" -#include "chrome/common/pref_names.h" -#include "chrome/common/url_constants.h" -#include "chrome/test/base/browser_with_test_window_test.h" -#include "chrome/test/base/testing_browser_process.h" -#include "chrome/test/base/testing_pref_service.h" -#include "chrome/test/base/testing_profile_manager.h" -#include "content/public/browser/web_contents.h" -#include "content/test/test_renderer_host.h" - -typedef BrowserWithTestWindowTest BrowserListTest; - -namespace { - -// Helper function to iterate and count all the tabs. -size_t CountAllTabs() { - size_t count = 0; - for (TabContentsIterator iterator; !iterator.done(); ++iterator) - ++count; - return count; -} - -// Helper function to navigate to the print preview page. -void NavigateToPrintUrl(TabContentsWrapper* tab, int page_id) { - content::RenderViewHostTester::For( - tab->web_contents()->GetRenderViewHost())->SendNavigate( - page_id, GURL(chrome::kChromeUIPrintURL)); -} - -} // namespace - -TEST_F(BrowserListTest, TabContentsIteratorVerifyCount) { - // Make sure we have 1 window to start with. - EXPECT_EQ(1U, BrowserList::size()); - - EXPECT_EQ(0U, CountAllTabs()); - - // Create more browsers/windows. - scoped_ptr browser2(new Browser(Browser::TYPE_TABBED, profile())); - scoped_ptr browser3(new Browser(Browser::TYPE_TABBED, profile())); - scoped_ptr browser4(new Browser(Browser::TYPE_TABBED, profile())); - - scoped_ptr window2(new TestBrowserWindow(browser2.get())); - scoped_ptr window3(new TestBrowserWindow(browser3.get())); - scoped_ptr window4(new TestBrowserWindow(browser4.get())); - - browser2->SetWindowForTesting(window2.get()); - browser3->SetWindowForTesting(window3.get()); - browser4->SetWindowForTesting(window4.get()); - - // Sanity checks. - EXPECT_EQ(4U, BrowserList::size()); - EXPECT_EQ(0, browser()->tab_count()); - EXPECT_EQ(0, browser2->tab_count()); - EXPECT_EQ(0, browser3->tab_count()); - EXPECT_EQ(0, browser4->tab_count()); - - EXPECT_EQ(0U, CountAllTabs()); - - // Add some tabs. - for (size_t i = 0; i < 3; ++i) - browser2->NewTab(); - browser3->NewTab(); - - EXPECT_EQ(4U, CountAllTabs()); - - // Close some tabs. - browser2->CloseAllTabs(); - - EXPECT_EQ(1U, CountAllTabs()); - - // Add lots of tabs. - for (size_t i = 0; i < 41; ++i) - browser()->NewTab(); - - EXPECT_EQ(42U, CountAllTabs()); - // Close all remaining tabs to keep all the destructors happy. - browser3->CloseAllTabs(); -} - -TEST_F(BrowserListTest, TabContentsIteratorVerifyBrowser) { - // Make sure we have 1 window to start with. - EXPECT_EQ(1U, BrowserList::size()); - - // Create more browsers/windows. - scoped_ptr browser2(new Browser(Browser::TYPE_TABBED, profile())); - scoped_ptr browser3(new Browser(Browser::TYPE_TABBED, profile())); - - scoped_ptr window2(new TestBrowserWindow(browser2.get())); - scoped_ptr window3(new TestBrowserWindow(browser3.get())); - - browser2->SetWindowForTesting(window2.get()); - browser3->SetWindowForTesting(window3.get()); - - // Sanity checks. - EXPECT_EQ(3U, BrowserList::size()); - EXPECT_EQ(0, browser()->tab_count()); - EXPECT_EQ(0, browser2->tab_count()); - EXPECT_EQ(0, browser3->tab_count()); - - EXPECT_EQ(0U, CountAllTabs()); - - // Add some tabs. - for (size_t i = 0; i < 3; ++i) - browser2->NewTab(); - browser3->NewTab(); - - size_t count = 0; - for (TabContentsIterator iterator; !iterator.done(); ++iterator, ++count) { - if (count < 3) - EXPECT_EQ(browser2.get(), iterator.browser()); - else if (count == 3) - EXPECT_EQ(browser3.get(), iterator.browser()); - else - EXPECT_TRUE(false); - } - - // Close some tabs. - browser2->CloseAllTabs(); - - count = 0; - for (TabContentsIterator iterator; !iterator.done(); ++iterator, ++count) { - if (count == 0) - EXPECT_EQ(browser3.get(), iterator.browser()); - else - EXPECT_TRUE(false); - } - - // Now make it one tab per browser. - browser()->NewTab(); - browser2->NewTab(); - - count = 0; - for (TabContentsIterator iterator; !iterator.done(); ++iterator, ++count) { - if (count == 0) - EXPECT_EQ(browser(), iterator.browser()); - else if (count == 1) - EXPECT_EQ(browser2.get(), iterator.browser()); - else if (count == 2) - EXPECT_EQ(browser3.get(), iterator.browser()); - else - EXPECT_TRUE(false); - } - - // Close all remaining tabs to keep all the destructors happy. - browser2->CloseAllTabs(); - browser3->CloseAllTabs(); -} - -#if 0 -// TODO(thestig) Fix or remove this test. http://crbug.com/100309 -TEST_F(BrowserListTest, TabContentsIteratorBackgroundPrinting) { - // Make sure we have 1 window to start with. - EXPECT_EQ(1U, BrowserList::size()); - - // Create more browsers/windows. - scoped_ptr browser2(new Browser(Browser::TYPE_TABBED, profile())); - scoped_ptr browser3(new Browser(Browser::TYPE_TABBED, profile())); - - scoped_ptr window2(new TestBrowserWindow(browser2.get())); - scoped_ptr window3(new TestBrowserWindow(browser3.get())); - - browser2->SetWindowForTesting(window2.get()); - browser3->SetWindowForTesting(window3.get()); - - EXPECT_EQ(0U, CountAllTabs()); - - // Add some tabs. - for (size_t i = 0; i < 3; ++i) - browser2->NewTab(); - browser3->NewTab(); - - EXPECT_EQ(4U, CountAllTabs()); - - TestingBrowserProcess* browser_process = - static_cast(g_browser_process); - printing::BackgroundPrintingManager* bg_print_manager = - browser_process->background_printing_manager(); - - // Grab a tab and give ownership to BackgroundPrintingManager. - TabContentsIterator tab_iterator; - TabContentsWrapper* tab = *tab_iterator; - int page_id = 1; - NavigateToPrintUrl(tab, page_id++); - - bg_print_manager->OwnPrintPreviewTab(tab); - - EXPECT_EQ(4U, CountAllTabs()); - - // Close remaining tabs. - browser2->CloseAllTabs(); - browser3->CloseAllTabs(); - - EXPECT_EQ(1U, CountAllTabs()); - - // Delete the last remaining tab. - delete tab; - - EXPECT_EQ(0U, CountAllTabs()); - - // Add some tabs. - for (size_t i = 0; i < 3; ++i) { - browser2->NewTab(); - browser3->NewTab(); - } - - EXPECT_EQ(6U, CountAllTabs()); - - // Tell BackgroundPrintingManager to take ownership of all tabs. - // Save the tabs in |owned_tabs| because manipulating tabs in the middle of - // TabContentsIterator is a bad idea. - std::vector owned_tabs; - for (TabContentsIterator iterator; !iterator.done(); ++iterator) { - NavigateToPrintUrl(*iterator, page_id++); - owned_tabs.push_back(*iterator); - } - for (std::vector::iterator it = owned_tabs.begin(); - it != owned_tabs.end(); ++it) { - bg_print_manager->OwnPrintPreviewTab(*it); - } - - EXPECT_EQ(6U, CountAllTabs()); - - // Delete all tabs to clean up. - for (std::vector::iterator it = owned_tabs.begin(); - it != owned_tabs.end(); ++it) { - delete *it; - } - - EXPECT_EQ(0U, CountAllTabs()); -} -#endif - -#if defined(OS_CHROMEOS) -// Calling AttemptRestart on ChromeOS will exit the test. -#define MAYBE_AttemptRestart DISABLED_AttemptRestart -#else -#define MAYBE_AttemptRestart AttemptRestart -#endif - -TEST_F(BrowserListTest, MAYBE_AttemptRestart) { - ASSERT_TRUE(g_browser_process); - TestingPrefService testing_pref_service; - testing_pref_service.RegisterBooleanPref(prefs::kWasRestarted, false); - testing_pref_service.RegisterBooleanPref(prefs::kRestartLastSessionOnShutdown, - false); - - TestingBrowserProcess* testing_browser_process = - static_cast(g_browser_process); - testing_browser_process->SetLocalState(&testing_pref_service); - ASSERT_TRUE(g_browser_process->local_state()); - ProfileManager* profile_manager = new ProfileManager(FilePath()); - testing_browser_process->SetProfileManager(profile_manager); - - browser::AttemptRestart(); - // Cancel the effects of us calling browser::AttemptRestart. Otherwise tests - // ran after this one will fail. - browser_shutdown::SetTryingToQuit(false); - - EXPECT_TRUE(testing_pref_service.GetBoolean(prefs::kWasRestarted)); - testing_browser_process->SetLocalState(NULL); -} diff --git a/chrome/browser/ui/cocoa/hung_renderer_controller.mm b/chrome/browser/ui/cocoa/hung_renderer_controller.mm index 5a51d25..40346bc 100644 --- a/chrome/browser/ui/cocoa/hung_renderer_controller.mm +++ b/chrome/browser/ui/cocoa/hung_renderer_controller.mm @@ -12,10 +12,10 @@ #include "base/sys_string_conversions.h" #include "chrome/browser/favicon/favicon_tab_helper.h" #include "chrome/browser/ui/browser_dialogs.h" -#include "chrome/browser/ui/browser_list.h" #import "chrome/browser/ui/cocoa/multi_key_equivalent_button.h" #import "chrome/browser/ui/cocoa/tab_contents/favicon_util.h" #include "chrome/browser/ui/tab_contents/core_tab_helper.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/common/logging_chrome.h" #include "content/public/browser/render_process_host.h" diff --git a/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc b/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc index 56a2e44..2861819 100644 --- a/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc +++ b/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc @@ -9,9 +9,9 @@ #include "base/process_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/favicon/favicon_tab_helper.h" -#include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/gtk/gtk_util.h" #include "chrome/browser/ui/tab_contents/core_tab_helper.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/common/logging_chrome.h" #include "content/public/browser/render_process_host.h" diff --git a/chrome/browser/ui/tab_contents/tab_contents_iterator.cc b/chrome/browser/ui/tab_contents/tab_contents_iterator.cc new file mode 100644 index 0000000..6f6ee0f --- /dev/null +++ b/chrome/browser/ui/tab_contents/tab_contents_iterator.cc @@ -0,0 +1,59 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" + +#include "base/logging.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/printing/background_printing_manager.h" + +namespace { + +printing::BackgroundPrintingManager* GetBackgroundPrintingManager() { + return g_browser_process->background_printing_manager(); +} + +} // namespace + +TabContentsIterator::TabContentsIterator() + : browser_iterator_(BrowserList::begin()), + web_view_index_(-1), + bg_printing_iterator_(GetBackgroundPrintingManager()->begin()), + cur_(NULL) { + Advance(); +} + +void TabContentsIterator::Advance() { + // The current WebContents should be valid unless we are at the beginning. + DCHECK(cur_ || (web_view_index_ == -1 && + browser_iterator_ == BrowserList::begin())) + << "Trying to advance past the end"; + + // Update cur_ to the next WebContents in the list. + while (browser_iterator_ != BrowserList::end()) { + if (++web_view_index_ >= (*browser_iterator_)->tab_count()) { + // Advance to the next Browser in the list. + ++browser_iterator_; + web_view_index_ = -1; + continue; + } + + TabContentsWrapper* next_tab = + (*browser_iterator_)->GetTabContentsWrapperAt(web_view_index_); + if (next_tab) { + cur_ = next_tab; + return; + } + } + // If no more WebContents from Browsers, check the BackgroundPrintingManager. + while (bg_printing_iterator_ != GetBackgroundPrintingManager()->end()) { + cur_ = *bg_printing_iterator_; + CHECK(cur_); + ++bg_printing_iterator_; + return; + } + // Reached the end - no more WebContents. + cur_ = NULL; +} diff --git a/chrome/browser/ui/tab_contents/tab_contents_iterator.h b/chrome/browser/ui/tab_contents/tab_contents_iterator.h new file mode 100644 index 0000000..4531c13 --- /dev/null +++ b/chrome/browser/ui/tab_contents/tab_contents_iterator.h @@ -0,0 +1,85 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_UI_TAB_CONTENTS_TAB_CONTENTS_ITERATOR_H_ +#define CHROME_BROWSER_UI_TAB_CONTENTS_TAB_CONTENTS_ITERATOR_H_ +#pragma once + +#include + +#include "base/basictypes.h" +#include "chrome/browser/ui/browser_list.h" + +class TabContentsWrapper; + +// Iterates through all web view hosts in all browser windows. Because the +// renderers act asynchronously, getting a host through this interface does +// not guarantee that the renderer is ready to go. Doing anything to affect +// browser windows or tabs while iterating may cause incorrect behavior. +// +// Example: +// for (TabContentsIterator iterator; !iterator.done(); ++iterator) { +// TabContentsWrapper* cur = *iterator; +// -or- +// iterator->operationOnTabContents(); +// ... +// } +class TabContentsIterator { + public: + TabContentsIterator(); + + // Returns true if we are past the last Browser. + bool done() const { return cur_ == NULL; } + + // Returns the Browser instance associated with the current + // TabContentsWrapper. Valid as long as !done() + Browser* browser() const { + if (browser_iterator_ != BrowserList::end()) + return *browser_iterator_; + return NULL; + } + + // Returns the current TabContentsWrapper, valid as long as !Done() + TabContentsWrapper* operator->() const { + return cur_; + } + TabContentsWrapper* operator*() const { + return cur_; + } + + // Incrementing operators, valid as long as !Done() + TabContentsWrapper* operator++() { // ++preincrement + Advance(); + return cur_; + } + TabContentsWrapper* operator++(int) { // postincrement++ + TabContentsWrapper* tmp = cur_; + Advance(); + return tmp; + } + + private: + // Loads the next host into Cur. This is designed so that for the initial + // call when browser_iterator_ points to the first browser and + // web_view_index_ is -1, it will fill the first host. + void Advance(); + + // Iterator over all the Browser objects. + BrowserList::const_iterator browser_iterator_; + + // tab index into the current Browser of the current web view + int web_view_index_; + + // iterator over the TabContentsWrappers doing background printing. + std::set::const_iterator bg_printing_iterator_; + + // Current TabContentsWrapper, or NULL if we're at the end of the list. This + // can be extracted given the browser iterator and index, but it's nice to + // cache this since the caller may access the current host many times. + TabContentsWrapper* cur_; + + DISALLOW_COPY_AND_ASSIGN(TabContentsIterator); +}; + +#endif // CHROME_BROWSER_UI_TAB_CONTENTS_TAB_CONTENTS_ITERATOR_H_ diff --git a/chrome/browser/ui/tab_contents/tab_contents_iterator_unittest.cc b/chrome/browser/ui/tab_contents/tab_contents_iterator_unittest.cc new file mode 100644 index 0000000..e49a7dd --- /dev/null +++ b/chrome/browser/ui/tab_contents/tab_contents_iterator_unittest.cc @@ -0,0 +1,274 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" + +#include "chrome/browser/browser_process.h" +#include "chrome/browser/browser_shutdown.h" +#include "chrome/browser/lifetime/application_lifetime.h" +#include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/printing/background_printing_manager.h" +#include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" +#include "chrome/common/pref_names.h" +#include "chrome/common/url_constants.h" +#include "chrome/test/base/browser_with_test_window_test.h" +#include "chrome/test/base/testing_browser_process.h" +#include "chrome/test/base/testing_pref_service.h" +#include "chrome/test/base/testing_profile_manager.h" +#include "content/public/browser/web_contents.h" +#include "content/test/test_renderer_host.h" + +typedef BrowserWithTestWindowTest BrowserListTest; + +namespace { + +// Helper function to iterate and count all the tabs. +size_t CountAllTabs() { + size_t count = 0; + for (TabContentsIterator iterator; !iterator.done(); ++iterator) + ++count; + return count; +} + +// Helper function to navigate to the print preview page. +void NavigateToPrintUrl(TabContentsWrapper* tab, int page_id) { + content::RenderViewHostTester::For( + tab->web_contents()->GetRenderViewHost())->SendNavigate( + page_id, GURL(chrome::kChromeUIPrintURL)); +} + +} // namespace + +TEST_F(BrowserListTest, TabContentsIteratorVerifyCount) { + // Make sure we have 1 window to start with. + EXPECT_EQ(1U, BrowserList::size()); + + EXPECT_EQ(0U, CountAllTabs()); + + // Create more browsers/windows. + scoped_ptr browser2(new Browser(Browser::TYPE_TABBED, profile())); + scoped_ptr browser3(new Browser(Browser::TYPE_TABBED, profile())); + scoped_ptr browser4(new Browser(Browser::TYPE_TABBED, profile())); + + scoped_ptr window2(new TestBrowserWindow(browser2.get())); + scoped_ptr window3(new TestBrowserWindow(browser3.get())); + scoped_ptr window4(new TestBrowserWindow(browser4.get())); + + browser2->SetWindowForTesting(window2.get()); + browser3->SetWindowForTesting(window3.get()); + browser4->SetWindowForTesting(window4.get()); + + // Sanity checks. + EXPECT_EQ(4U, BrowserList::size()); + EXPECT_EQ(0, browser()->tab_count()); + EXPECT_EQ(0, browser2->tab_count()); + EXPECT_EQ(0, browser3->tab_count()); + EXPECT_EQ(0, browser4->tab_count()); + + EXPECT_EQ(0U, CountAllTabs()); + + // Add some tabs. + for (size_t i = 0; i < 3; ++i) + browser2->NewTab(); + browser3->NewTab(); + + EXPECT_EQ(4U, CountAllTabs()); + + // Close some tabs. + browser2->CloseAllTabs(); + + EXPECT_EQ(1U, CountAllTabs()); + + // Add lots of tabs. + for (size_t i = 0; i < 41; ++i) + browser()->NewTab(); + + EXPECT_EQ(42U, CountAllTabs()); + // Close all remaining tabs to keep all the destructors happy. + browser3->CloseAllTabs(); +} + +TEST_F(BrowserListTest, TabContentsIteratorVerifyBrowser) { + // Make sure we have 1 window to start with. + EXPECT_EQ(1U, BrowserList::size()); + + // Create more browsers/windows. + scoped_ptr browser2(new Browser(Browser::TYPE_TABBED, profile())); + scoped_ptr browser3(new Browser(Browser::TYPE_TABBED, profile())); + + scoped_ptr window2(new TestBrowserWindow(browser2.get())); + scoped_ptr window3(new TestBrowserWindow(browser3.get())); + + browser2->SetWindowForTesting(window2.get()); + browser3->SetWindowForTesting(window3.get()); + + // Sanity checks. + EXPECT_EQ(3U, BrowserList::size()); + EXPECT_EQ(0, browser()->tab_count()); + EXPECT_EQ(0, browser2->tab_count()); + EXPECT_EQ(0, browser3->tab_count()); + + EXPECT_EQ(0U, CountAllTabs()); + + // Add some tabs. + for (size_t i = 0; i < 3; ++i) + browser2->NewTab(); + browser3->NewTab(); + + size_t count = 0; + for (TabContentsIterator iterator; !iterator.done(); ++iterator, ++count) { + if (count < 3) + EXPECT_EQ(browser2.get(), iterator.browser()); + else if (count == 3) + EXPECT_EQ(browser3.get(), iterator.browser()); + else + EXPECT_TRUE(false); + } + + // Close some tabs. + browser2->CloseAllTabs(); + + count = 0; + for (TabContentsIterator iterator; !iterator.done(); ++iterator, ++count) { + if (count == 0) + EXPECT_EQ(browser3.get(), iterator.browser()); + else + EXPECT_TRUE(false); + } + + // Now make it one tab per browser. + browser()->NewTab(); + browser2->NewTab(); + + count = 0; + for (TabContentsIterator iterator; !iterator.done(); ++iterator, ++count) { + if (count == 0) + EXPECT_EQ(browser(), iterator.browser()); + else if (count == 1) + EXPECT_EQ(browser2.get(), iterator.browser()); + else if (count == 2) + EXPECT_EQ(browser3.get(), iterator.browser()); + else + EXPECT_TRUE(false); + } + + // Close all remaining tabs to keep all the destructors happy. + browser2->CloseAllTabs(); + browser3->CloseAllTabs(); +} + +#if 0 +// TODO(thestig) Fix or remove this test. http://crbug.com/100309 +TEST_F(BrowserListTest, TabContentsIteratorBackgroundPrinting) { + // Make sure we have 1 window to start with. + EXPECT_EQ(1U, BrowserList::size()); + + // Create more browsers/windows. + scoped_ptr browser2(new Browser(Browser::TYPE_TABBED, profile())); + scoped_ptr browser3(new Browser(Browser::TYPE_TABBED, profile())); + + scoped_ptr window2(new TestBrowserWindow(browser2.get())); + scoped_ptr window3(new TestBrowserWindow(browser3.get())); + + browser2->SetWindowForTesting(window2.get()); + browser3->SetWindowForTesting(window3.get()); + + EXPECT_EQ(0U, CountAllTabs()); + + // Add some tabs. + for (size_t i = 0; i < 3; ++i) + browser2->NewTab(); + browser3->NewTab(); + + EXPECT_EQ(4U, CountAllTabs()); + + TestingBrowserProcess* browser_process = + static_cast(g_browser_process); + printing::BackgroundPrintingManager* bg_print_manager = + browser_process->background_printing_manager(); + + // Grab a tab and give ownership to BackgroundPrintingManager. + TabContentsIterator tab_iterator; + TabContentsWrapper* tab = *tab_iterator; + int page_id = 1; + NavigateToPrintUrl(tab, page_id++); + + bg_print_manager->OwnPrintPreviewTab(tab); + + EXPECT_EQ(4U, CountAllTabs()); + + // Close remaining tabs. + browser2->CloseAllTabs(); + browser3->CloseAllTabs(); + + EXPECT_EQ(1U, CountAllTabs()); + + // Delete the last remaining tab. + delete tab; + + EXPECT_EQ(0U, CountAllTabs()); + + // Add some tabs. + for (size_t i = 0; i < 3; ++i) { + browser2->NewTab(); + browser3->NewTab(); + } + + EXPECT_EQ(6U, CountAllTabs()); + + // Tell BackgroundPrintingManager to take ownership of all tabs. + // Save the tabs in |owned_tabs| because manipulating tabs in the middle of + // TabContentsIterator is a bad idea. + std::vector owned_tabs; + for (TabContentsIterator iterator; !iterator.done(); ++iterator) { + NavigateToPrintUrl(*iterator, page_id++); + owned_tabs.push_back(*iterator); + } + for (std::vector::iterator it = owned_tabs.begin(); + it != owned_tabs.end(); ++it) { + bg_print_manager->OwnPrintPreviewTab(*it); + } + + EXPECT_EQ(6U, CountAllTabs()); + + // Delete all tabs to clean up. + for (std::vector::iterator it = owned_tabs.begin(); + it != owned_tabs.end(); ++it) { + delete *it; + } + + EXPECT_EQ(0U, CountAllTabs()); +} +#endif + +#if defined(OS_CHROMEOS) +// Calling AttemptRestart on ChromeOS will exit the test. +#define MAYBE_AttemptRestart DISABLED_AttemptRestart +#else +#define MAYBE_AttemptRestart AttemptRestart +#endif + +TEST_F(BrowserListTest, MAYBE_AttemptRestart) { + ASSERT_TRUE(g_browser_process); + TestingPrefService testing_pref_service; + testing_pref_service.RegisterBooleanPref(prefs::kWasRestarted, false); + testing_pref_service.RegisterBooleanPref(prefs::kRestartLastSessionOnShutdown, + false); + + TestingBrowserProcess* testing_browser_process = + static_cast(g_browser_process); + testing_browser_process->SetLocalState(&testing_pref_service); + ASSERT_TRUE(g_browser_process->local_state()); + ProfileManager* profile_manager = new ProfileManager(FilePath()); + testing_browser_process->SetProfileManager(profile_manager); + + browser::AttemptRestart(); + // Cancel the effects of us calling browser::AttemptRestart. Otherwise tests + // ran after this one will fail. + browser_shutdown::SetTryingToQuit(false); + + EXPECT_TRUE(testing_pref_service.GetBoolean(prefs::kWasRestarted)); + testing_browser_process->SetLocalState(NULL); +} diff --git a/chrome/browser/ui/views/hung_renderer_view.cc b/chrome/browser/ui/views/hung_renderer_view.cc index 3e8428c..59146a3 100644 --- a/chrome/browser/ui/views/hung_renderer_view.cc +++ b/chrome/browser/ui/views/hung_renderer_view.cc @@ -14,8 +14,8 @@ #include "base/utf_string_conversions.h" #include "chrome/browser/favicon/favicon_tab_helper.h" #include "chrome/browser/platform_util.h" -#include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/tab_contents/core_tab_helper.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/logging_chrome.h" diff --git a/chrome/browser/ui/webui/inspect_ui.cc b/chrome/browser/ui/webui/inspect_ui.cc index 69e19db..e0e79c4 100644 --- a/chrome/browser/ui/webui/inspect_ui.cc +++ b/chrome/browser/ui/webui/inspect_ui.cc @@ -17,17 +17,17 @@ #include "chrome/browser/debugger/devtools_window.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" -#include "chrome/browser/ui/webui/chrome_url_data_manager_backend.h" #include "chrome/browser/ui/webui/chrome_url_data_manager.h" +#include "chrome/browser/ui/webui/chrome_url_data_manager_backend.h" #include "chrome/browser/ui/webui/chrome_web_ui_data_source.h" #include "chrome/common/url_constants.h" +#include "content/public/browser/browser_thread.h" #include "content/public/browser/child_process_data.h" #include "content/public/browser/devtools_agent_host_registry.h" #include "content/public/browser/devtools_client_host.h" #include "content/public/browser/devtools_manager.h" -#include "content/public/browser/browser_thread.h" #include "content/public/browser/favicon_status.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/notification_service.h" @@ -38,10 +38,9 @@ #include "content/public/browser/render_widget_host.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_ui.h" +#include "content/public/browser/web_ui_message_handler.h" #include "content/public/browser/worker_service.h" #include "content/public/browser/worker_service_observer.h" -#include "content/public/browser/web_contents.h" -#include "content/public/browser/web_ui_message_handler.h" #include "content/public/common/process_type.h" #include "grit/browser_resources.h" #include "grit/generated_resources.h" diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index f59d3d3..a8e5cf6 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -3222,6 +3222,8 @@ 'browser/ui/tab_contents/core_tab_helper.h', 'browser/ui/tab_contents/core_tab_helper_delegate.cc', 'browser/ui/tab_contents/core_tab_helper_delegate.h', + 'browser/ui/tab_contents/tab_contents_iterator.cc', + 'browser/ui/tab_contents/tab_contents_iterator.h', 'browser/ui/tab_contents/tab_contents_wrapper.cc', 'browser/ui/tab_contents/tab_contents_wrapper.h', 'browser/ui/tab_modal_confirm_dialog_delegate.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index ab9a9d0..b807b4f 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1570,7 +1570,6 @@ 'browser/task_profiler/task_profiler_data_serializer_unittest.cc', 'browser/themes/browser_theme_pack_unittest.cc', 'browser/themes/theme_service_unittest.cc', - 'browser/ui/browser_list_unittest.cc', 'browser/ui/browser_unittest.cc', # It is safe to list */cocoa/* files in the "common" file list # without an explicit exclusion since gyp is smart enough to @@ -1750,6 +1749,7 @@ 'browser/ui/panels/panel_mouse_watcher_unittest.cc', 'browser/ui/search_engines/keyword_editor_controller_unittest.cc', 'browser/ui/select_file_dialog_unittest.cc', + 'browser/ui/tab_contents/tab_contents_iterator_unittest.cc', 'browser/ui/tabs/dock_info_unittest.cc', 'browser/ui/tabs/pinned_tab_codec_unittest.cc', 'browser/ui/tabs/pinned_tab_service_unittest.cc', @@ -2343,8 +2343,8 @@ 'browser/sync/profile_sync_service_session_unittest.cc', 'browser/sync/sync_global_error_unittest.cc', 'browser/sync/sync_setup_wizard_unittest.cc', - 'browser/ui/browser_list_unittest.cc', 'browser/ui/browser_unittest.cc', + 'browser/ui/tab_contents/tab_contents_iterator_unittest.cc', 'browser/ui/toolbar/toolbar_model_unittest.cc', 'browser/ui/toolbar/wrench_menu_model_unittest.cc', 'browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc', -- cgit v1.1