diff options
| -rw-r--r-- | chrome/browser/cocoa/location_bar_view_mac.mm | 41 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_browsertest.cc | 93 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_browsertest.h | 10 | ||||
| -rw-r--r-- | chrome/browser/extensions/page_action_apitest.cc | 64 | ||||
| -rw-r--r-- | chrome/browser/gtk/location_bar_view_gtk.cc | 11 | ||||
| -rw-r--r-- | chrome/browser/views/location_bar_view.cc | 14 | ||||
| -rw-r--r-- | chrome/common/notification_type.h | 5 |
7 files changed, 123 insertions, 115 deletions
diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm index eca15f9..fca4078 100644 --- a/chrome/browser/cocoa/location_bar_view_mac.mm +++ b/chrome/browser/cocoa/location_bar_view_mac.mm @@ -154,12 +154,26 @@ void LocationBarViewMac::UpdateContentBlockedIcons() { } void LocationBarViewMac::UpdatePageActions() { + size_t count_before = page_action_views_.Count(); page_action_views_.RefreshViews(); [field_ resetFieldEditorFrameIfNeeded]; + if (page_action_views_.Count() != count_before) { + NotificationService::current()->Notify( + NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED, + Source<LocationBar>(this), + NotificationService::NoDetails()); + } } void LocationBarViewMac::InvalidatePageActions() { + size_t count_before = page_action_views_.Count(); page_action_views_.DeleteAll(); + if (page_action_views_.Count() != count_before) { + NotificationService::current()->Notify( + NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED, + Source<LocationBar>(this), + NotificationService::NoDetails()); + } } void LocationBarViewMac::SaveStateToContents(TabContents* contents) { @@ -353,11 +367,6 @@ void LocationBarViewMac::SetPreviewEnabledPageAction( page_action_image_view->set_preview_enabled(preview_enabled); page_action_image_view->UpdateVisibility(contents, GURL(WideToUTF8(toolbar_model_->GetText()))); - - NotificationService::current()->Notify( - NotificationType::EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED, - Source<ExtensionAction>(page_action), - Details<TabContents>(contents)); } size_t LocationBarViewMac::GetPageActionIndex(ExtensionAction* page_action) { @@ -722,7 +731,13 @@ void LocationBarViewMac::PageActionImageView::UpdateVisibility( if (!skia_icon.isNull()) SetImage(gfx::SkBitmapToNSImage(skia_icon)); } - SetVisible(visible); + if (IsVisible() != visible) { + SetVisible(visible); + NotificationService::current()->Notify( + NotificationType::EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED, + Source<ExtensionAction>(page_action_), + Details<TabContents>(contents)); + } } void LocationBarViewMac::PageActionImageView::SetToolTip(NSString* tooltip) { @@ -809,20 +824,8 @@ void LocationBarViewMac::PageActionViewList::RefreshViews() { return; GURL url = GURL(WideToUTF8(toolbar_model_->GetText())); - for (size_t i = 0; i < views_.size(); ++i) { + for (size_t i = 0; i < views_.size(); ++i) views_[i]->UpdateVisibility(contents, url); - // Fire the notification regardless of the visibility changes to trigger a - // redraw and tooltip update. - // TODO(andybons): This notification is fired on all platforms but never - // actually used outside of the Mac implementation and one unit test. Either - // update it to reflect a change to the extension view as a whole, or remove - // it. http://crbug.com/34339 - ExtensionAction* action = views_[i]->page_action(); - NotificationService::current()->Notify( - NotificationType::EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED, - Source<ExtensionAction>(action), - Details<TabContents>(contents)); - } } LocationBarViewMac::PageActionImageView* diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc index 4e25a8a..7bb8084 100644 --- a/chrome/browser/extensions/extension_browsertest.cc +++ b/chrome/browser/extensions/extension_browsertest.cc @@ -30,6 +30,11 @@ // minimize false positives. static const int kTimeoutMs = 60 * 1000; // 1 minute +ExtensionBrowserTest::ExtensionBrowserTest() + : target_page_action_count_(-1), + target_visible_page_action_count_(-1) { +} + void ExtensionBrowserTest::SetUpCommandLine(CommandLine* command_line) { // This enables DOM automation for tab contentses. EnableDOMAutomation(); @@ -161,49 +166,37 @@ void ExtensionBrowserTest::UninstallExtension(const std::string& extension_id) { } bool ExtensionBrowserTest::WaitForPageActionCountChangeTo(int count) { - base::Time start_time = base::Time::Now(); - while (true) { - LocationBarTesting* loc_bar = - browser()->window()->GetLocationBar()->GetLocationBarForTesting(); - - int actions = loc_bar->PageActionCount(); - if (actions == count) - return true; - - if ((base::Time::Now() - start_time).InMilliseconds() > kTimeoutMs) { - std::cout << "Timed out waiting for page actions to (un)load.\n" - << "Currently loaded page actions: " << IntToString(actions) - << "\n"; - return false; - } + NotificationRegistrar registrar; + registrar.Add(this, + NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED, + NotificationService::AllSources()); - MessageLoop::current()->PostDelayedTask(FROM_HERE, - new MessageLoop::QuitTask, 200); + MessageLoop::current()->PostDelayedTask(FROM_HERE, new MessageLoop::QuitTask, + kTimeoutMs); + + target_page_action_count_ = count; + LocationBarTesting* location_bar = + browser()->window()->GetLocationBar()->GetLocationBarForTesting(); + if (location_bar->PageActionCount() != count) ui_test_utils::RunMessageLoop(); - } + return location_bar->PageActionCount() == count; } bool ExtensionBrowserTest::WaitForPageActionVisibilityChangeTo(int count) { - base::Time start_time = base::Time::Now(); - while (true) { - LocationBarTesting* loc_bar = - browser()->window()->GetLocationBar()->GetLocationBarForTesting(); - - int visible = loc_bar->PageActionVisibleCount(); - if (visible == count) - return true; - - if ((base::Time::Now() - start_time).InMilliseconds() > kTimeoutMs) { - std::cout << "Timed out waiting for page actions to become (in)visible.\n" - << "Currently visible page actions: " << IntToString(visible) - << "\n"; - return false; - } + NotificationRegistrar registrar; + registrar.Add(this, + NotificationType::EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED, + NotificationService::AllSources()); - MessageLoop::current()->PostDelayedTask(FROM_HERE, - new MessageLoop::QuitTask, 200); + MessageLoop::current()->PostDelayedTask(FROM_HERE, new MessageLoop::QuitTask, + kTimeoutMs); + + target_visible_page_action_count_ = count; + LocationBarTesting* location_bar = + browser()->window()->GetLocationBar()->GetLocationBarForTesting(); + if (location_bar->PageActionVisibleCount() != count) ui_test_utils::RunMessageLoop(); - } + return location_bar->PageActionVisibleCount() == count; } bool ExtensionBrowserTest::WaitForExtensionHostsToLoad() { @@ -315,6 +308,34 @@ void ExtensionBrowserTest::Observe(NotificationType type, MessageLoopForUI::current()->Quit(); break; + case NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED: { + LocationBarTesting* location_bar = + browser()->window()->GetLocationBar()->GetLocationBarForTesting(); + std::cout << "Got EXTENSION_PAGE_ACTION_COUNT_CHANGED " + << "notification. Number of page actions: " + << location_bar->PageActionCount() << "\n"; + if (location_bar->PageActionCount() == + target_page_action_count_) { + target_page_action_count_ = -1; + MessageLoopForUI::current()->Quit(); + } + break; + } + + case NotificationType::EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED: { + LocationBarTesting* location_bar = + browser()->window()->GetLocationBar()->GetLocationBarForTesting(); + std::cout << "Got EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED " + << "notification. Number of visible page actions: " + << location_bar->PageActionVisibleCount() << "\n"; + if (location_bar->PageActionVisibleCount() == + target_visible_page_action_count_) { + target_visible_page_action_count_ = -1; + MessageLoopForUI::current()->Quit(); + } + break; + } + default: NOTREACHED(); break; diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h index c0534d1..a284607 100644 --- a/chrome/browser/extensions/extension_browsertest.h +++ b/chrome/browser/extensions/extension_browsertest.h @@ -20,6 +20,8 @@ class ExtensionBrowserTest : public InProcessBrowserTest, public NotificationObserver { protected: + ExtensionBrowserTest(); + virtual void SetUpCommandLine(CommandLine* command_line); bool LoadExtension(const FilePath& path); @@ -89,6 +91,14 @@ class ExtensionBrowserTest int expected_change); bool WaitForExtensionHostsToLoad(); + + // When waiting for page action count to change, we wait until it reaches this + // value. + int target_page_action_count_; + + // When waiting for visible page action count to change, we wait until it + // reaches this value. + int target_visible_page_action_count_; }; #endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSERTEST_H_ diff --git a/chrome/browser/extensions/page_action_apitest.cc b/chrome/browser/extensions/page_action_apitest.cc index 5b42890..c165623 100644 --- a/chrome/browser/extensions/page_action_apitest.cc +++ b/chrome/browser/extensions/page_action_apitest.cc @@ -157,74 +157,18 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, OldPageActions) { } } -class PageActionPopupTest : public ExtensionApiTest { -public: - bool RunExtensionTest(const char* extension_name) { - last_action_ = NULL; - last_visibility_ = false; - waiting_ = false; - - return ExtensionApiTest::RunExtensionTest(extension_name); - }; - - void Observe(NotificationType type, const NotificationSource& source, - const NotificationDetails& details) { - switch (type.value) { - case NotificationType::EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED: { - last_action_ = Source<ExtensionAction>(source).ptr(); - TabContents* contents = Details<TabContents>(details).ptr(); - int tab_id = ExtensionTabUtil::GetTabId(contents); - last_visibility_ = last_action_->GetIsVisible(tab_id); - if (waiting_) - MessageLoopForUI::current()->Quit(); - break; - } - default: - ExtensionBrowserTest::Observe(type, source, details); - break; - } - } - - void WaitForPopupVisibilityChange() { - // Wait for EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED to come in. - if (!last_action_) { - waiting_ = true; - ui_test_utils::RunMessageLoop(); - waiting_ = false; - } - } - - protected: - bool waiting_; - ExtensionAction* last_action_; - bool last_visibility_; -}; - // Tests popups in page actions. -IN_PROC_BROWSER_TEST_F(PageActionPopupTest, Show) { - NotificationRegistrar registrar; - registrar.Add(this, - NotificationType::EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED, - NotificationService::AllSources()); - +IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ShowPageActionPopup) { ASSERT_TRUE(RunExtensionTest("page_action/popup")) << message_; Extension* extension = GetSingleLoadedExtension(); ASSERT_TRUE(extension) << message_; - // Wait for The page action to actually become visible. - if (!last_visibility_) - last_action_ = NULL; - WaitForPopupVisibilityChange(); - ASSERT_TRUE(last_visibility_); - - LocationBarTesting* location_bar = - browser()->window()->GetLocationBar()->GetLocationBarForTesting(); - ASSERT_EQ(1, location_bar->PageActionVisibleCount()); - ExtensionAction* action = location_bar->GetVisiblePageAction(0); - EXPECT_EQ(action, last_action_); + ASSERT_TRUE(WaitForPageActionVisibilityChangeTo(1)); { ResultCatcher catcher; + LocationBarTesting* location_bar = + browser()->window()->GetLocationBar()->GetLocationBarForTesting(); location_bar->TestPageActionPressed(0); ASSERT_TRUE(catcher.GetNextResult()); } diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc index 00384e1..66af2aa 100644 --- a/chrome/browser/gtk/location_bar_view_gtk.cc +++ b/chrome/browser/gtk/location_bar_view_gtk.cc @@ -566,6 +566,10 @@ void LocationBarViewGtk::UpdatePageActions() { gtk_box_pack_end(GTK_BOX(page_action_hbox_.get()), page_action_views_[i]->widget(), FALSE, FALSE, 0); } + NotificationService::current()->Notify( + NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED, + Source<LocationBar>(this), + NotificationService::NoDetails()); } TabContents* contents = browser_->GetSelectedTabContents(); @@ -585,7 +589,14 @@ void LocationBarViewGtk::UpdatePageActions() { } void LocationBarViewGtk::InvalidatePageActions() { + size_t count_before = page_action_views_.size(); page_action_views_.reset(); + if (page_action_views_.size() != count_before) { + NotificationService::current()->Notify( + NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED, + Source<LocationBar>(this), + NotificationService::NoDetails()); + } } void LocationBarViewGtk::SaveStateToContents(TabContents* contents) { diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc index 5a29b59..eb8bd88 100644 --- a/chrome/browser/views/location_bar_view.cc +++ b/chrome/browser/views/location_bar_view.cc @@ -317,14 +317,28 @@ void LocationBarView::UpdateContentBlockedIcons() { } void LocationBarView::UpdatePageActions() { + size_t count_before = page_action_views_.size(); RefreshPageActionViews(); + if (page_action_views_.size() != count_before) { + NotificationService::current()->Notify( + NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED, + Source<LocationBar>(this), + NotificationService::NoDetails()); + } Layout(); SchedulePaint(); } void LocationBarView::InvalidatePageActions() { + size_t count_before = page_action_views_.size(); DeletePageActionViews(); + if (page_action_views_.size() != count_before) { + NotificationService::current()->Notify( + NotificationType::EXTENSION_PAGE_ACTION_COUNT_CHANGED, + Source<LocationBar>(this), + NotificationService::NoDetails()); + } } void LocationBarView::Focus() { diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h index ce5b8c4..6b4c802 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -749,6 +749,11 @@ class NotificationType { // ExtensionAction* that changed. There are no details. EXTENSION_BROWSER_ACTION_UPDATED, + // Sent when the count of page actions has changed. Note that some of them + // may not apply to the current page. The source is a LocationBar*. There + // are no details. + EXTENSION_PAGE_ACTION_COUNT_CHANGED, + // Sent when a page action's visibility has changed. The source is the // ExtensionAction* that changed. The details are a TabContents*. EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED, |