diff options
8 files changed, 218 insertions, 144 deletions
diff --git a/chrome/browser/blocked_popup_container.cc b/chrome/browser/blocked_popup_container.cc index c82998b..4744fb5 100644 --- a/chrome/browser/blocked_popup_container.cc +++ b/chrome/browser/blocked_popup_container.cc @@ -58,7 +58,7 @@ void BlockedPopupContainer::AddTabContents(TabContents* tab_contents, unblocked_popups_[tab_contents] = host; } else { - if (blocked_popups_.size() >= kImpossibleNumberOfPopups) { + if (blocked_popups_.size() == (kImpossibleNumberOfPopups - 1)) { delete tab_contents; LOG(INFO) << "Warning: Renderer is sending more popups to us than should " "be possible. Renderer compromised?"; @@ -125,8 +125,9 @@ void BlockedPopupContainer::AddBlockedNotice(const GURL& url, owner_->PopupNotificationVisibilityChanged(true); } -void BlockedPopupContainer::GetHostAndReasonForNotice( - size_t index, std::string* host, string16* reason) const { +void BlockedPopupContainer::GetHostAndReasonForNotice(size_t index, + std::string* host, + string16* reason) const { DCHECK(host); DCHECK(reason); *host = blocked_notices_[index].url_.host(); @@ -142,8 +143,7 @@ size_t BlockedPopupContainer::GetBlockedNoticeCount() const { } bool BlockedPopupContainer::IsHostWhitelisted(size_t index) const { - PopupHosts::const_iterator i(ConvertHostIndexToIterator(index)); - return (i == popup_hosts_.end()) ? false : i->second; + return ConvertHostIndexToIterator(index)->second; } void BlockedPopupContainer::ToggleWhitelistingForHost(size_t index) { diff --git a/chrome/browser/blocked_popup_container.h b/chrome/browser/blocked_popup_container.h index 155d648..7eb6192 100644 --- a/chrome/browser/blocked_popup_container.h +++ b/chrome/browser/blocked_popup_container.h @@ -114,12 +114,13 @@ class BlockedPopupContainer : public TabContentsDelegate, // Returns the number of blocked notices, popups don't count. size_t GetBlockedNoticeCount() const; - // Returns true if host |index| is whitelisted. Returns false if |index| is - // invalid. + // Returns true if host |index| is whitelisted. + // NOTE: Does not sanity-check; do not pass an invalid index! bool IsHostWhitelisted(size_t index) const; // If host |index| is currently whitelisted, un-whitelists it. Otherwise, // whitelists it and opens all blocked popups from it. + // NOTE: Does not sanity-check; do not pass an invalid index! void ToggleWhitelistingForHost(size_t index); // Deletes all popups and hides the interface parts. diff --git a/chrome/browser/blocked_popup_container_unittest.cc b/chrome/browser/blocked_popup_container_unittest.cc index cedf1c8..e57fec4 100644 --- a/chrome/browser/blocked_popup_container_unittest.cc +++ b/chrome/browser/blocked_popup_container_unittest.cc @@ -96,6 +96,6 @@ TEST_F(BlockedPopupContainerTest, BasicCase) { EXPECT_EQ(container_->GetBlockedPopupCount(), static_cast<size_t>(1)); EXPECT_EQ(container_->GetTabContentsAt(0), popup); + ASSERT_THAT(container_->GetHosts(), testing::Contains(host1)); EXPECT_FALSE(container_->IsHostWhitelisted(0)); - EXPECT_THAT(container_->GetHosts(), testing::Contains(host1)); } diff --git a/chrome/browser/cocoa/blocked_popup_container_controller.mm b/chrome/browser/cocoa/blocked_popup_container_controller.mm index 29e22e9..f84bb3a 100644 --- a/chrome/browser/cocoa/blocked_popup_container_controller.mm +++ b/chrome/browser/cocoa/blocked_popup_container_controller.mm @@ -197,13 +197,17 @@ class BlockedPopupContainerViewBridge : public BlockedPopupContainerView { } - (void)update { - size_t blockedPopups = container_->GetBlockedPopupCount(); + size_t blockedNotices = container_->GetBlockedNoticeCount(); + size_t blockedItems = container_->GetBlockedPopupCount() + blockedNotices; NSString* label = nil; - if (blockedPopups) { + if (!blockedItems) { + label = l10n_util::GetNSString(IDS_POPUPS_UNBLOCKED); + } else if (!blockedNotices) { label = l10n_util::GetNSStringF(IDS_POPUPS_BLOCKED_COUNT, - UintToString16(blockedPopups)); + UintToString16(blockedItems)); } else { - label = l10n_util::GetNSString(IDS_POPUPS_UNBLOCKED); + label = l10n_util::GetNSStringF(IDS_BLOCKED_NOTICE_COUNT, + UintToString16(blockedItems)); } [self resizeWithLabel:label]; [view_ setContent:label]; @@ -211,17 +215,31 @@ class BlockedPopupContainerViewBridge : public BlockedPopupContainerView { // Called when the user selects an item from the popup menu. The tag, if below // |kImpossibleNumberOfPopups| will be the index into the container's popup -// array. In that case, we should display the popup. If >= -// |kImpossibleNumberOfPopups|, it represents a host that we should whitelist. +// array. In that case, we should display the popup. Otherwise, the tag is +// either a host we should toggle whitelisting on or a notice (for which we do +// nothing as yet). // |sender| is the NSMenuItem that was chosen. - (void)menuAction:(id)sender { size_t tag = static_cast<size_t>([sender tag]); + + // Is this a click on a popup? if (tag < BlockedPopupContainer::kImpossibleNumberOfPopups) { container_->LaunchPopupAtIndex(tag); - } else { - size_t hostIndex = tag - BlockedPopupContainer::kImpossibleNumberOfPopups; - container_->ToggleWhitelistingForHost(hostIndex); + return; + } + + tag -= BlockedPopupContainer::kImpossibleNumberOfPopups; + + // Is this a click on a host? + size_t hostCount = container_->GetPopupHostCount(); + if (tag < hostCount) { + container_->ToggleWhitelistingForHost(tag); + return; } + + tag -= hostCount; + + // Nothing to do for now for notices. } namespace { @@ -280,17 +298,38 @@ void GetURLAndTitleForPopup( std::vector<std::string> hosts(container_->GetHosts()); if (!hosts.empty() && count) [menu addItem:[NSMenuItem separatorItem]]; + size_t first_host = BlockedPopupContainer::kImpossibleNumberOfPopups; for (size_t i = 0; i < hosts.size(); ++i) { NSString* titleStr = - l10n_util::GetNSStringF(IDS_POPUP_HOST_FORMAT, - UTF8ToUTF16(hosts[i])); + l10n_util::GetNSStringF(IDS_POPUP_HOST_FORMAT, UTF8ToUTF16(hosts[i])); scoped_nsobject<NSMenuItem> item( [[NSMenuItem alloc] initWithTitle:titleStr action:@selector(menuAction:) keyEquivalent:@""]); if (container_->IsHostWhitelisted(i)) [item setState:NSOnState]; - [item setTag:BlockedPopupContainer::kImpossibleNumberOfPopups + i]; + [item setTag:first_host + i]; + [item setTarget:self]; + [menu addItem:item.get()]; + } + + // Add the list of notices. We begin tagging these at + // |kImpossibleNumberOfPopups + hosts.size()|. + size_t notice_count = container_->GetBlockedNoticeCount(); + if (notice_count && (!hosts.empty() || count)) + [menu addItem:[NSMenuItem separatorItem]]; + size_t first_notice = first_host + hosts.size(); + for (size_t i = 0; i < notice_count; ++i) { + std::string host; + string16 reason; + container_->GetHostAndReasonForNotice(i, &host, &reason); + NSString* titleStr = + l10n_util::GetNSStringF(IDS_NOTICE_TITLE_FORMAT, UTF8ToUTF16(host)); + scoped_nsobject<NSMenuItem> item( + [[NSMenuItem alloc] initWithTitle:titleStr + action:@selector(menuAction:) + keyEquivalent:@""]); + [item setTag:first_notice + i]; [item setTarget:self]; [menu addItem:item.get()]; } diff --git a/chrome/browser/cocoa/blocked_popup_container_controller_unittest.mm b/chrome/browser/cocoa/blocked_popup_container_controller_unittest.mm index 54c6ca0..40a3f3bd 100644 --- a/chrome/browser/cocoa/blocked_popup_container_controller_unittest.mm +++ b/chrome/browser/cocoa/blocked_popup_container_controller_unittest.mm @@ -73,8 +73,9 @@ TEST_F(BlockedPopupContainerControllerTest, BasicPopupBlock) { popup->controller().LoadURL(GetTestCase("error"), GURL(), PageTransition::LINK); container_->AddTabContents(popup, gfx::Rect(), host1); - EXPECT_EQ(container_->GetBlockedPopupCount(), static_cast<size_t>(1)); - EXPECT_EQ(container_->GetTabContentsAt(0), popup); + EXPECT_EQ(1U, container_->GetBlockedPopupCount()); + EXPECT_EQ(popup, container_->GetTabContentsAt(0)); + ASSERT_EQ(1U, container_->GetPopupHostCount()); EXPECT_FALSE(container_->IsHostWhitelisted(0)); // Ensure the view has been displayed. If it has a superview, then ShowView() @@ -82,20 +83,20 @@ TEST_F(BlockedPopupContainerControllerTest, BasicPopupBlock) { // UpdateLabel() has been called. EXPECT_TRUE([cocoa_controller_ view]); EXPECT_TRUE([[cocoa_controller_ view] superview]); - EXPECT_TRUE([[[cocoa_controller_ view] content] length] > 0); + EXPECT_GT([[[cocoa_controller_ view] content] length], 0U); // Validate the menu. It should have 4 items (the dummy title item, 1 poupup, // a separator, 1 host). NSMenu* menu = [cocoa_controller_ buildMenu]; EXPECT_TRUE(menu); - EXPECT_EQ([menu numberOfItems], 4); + EXPECT_EQ(4, [menu numberOfItems]); // Change the whitelisting and make sure the host is checked. container_->ToggleWhitelistingForHost(0); menu = [cocoa_controller_ buildMenu]; EXPECT_TRUE(menu); - EXPECT_EQ([menu numberOfItems], 2); - EXPECT_EQ([[menu itemAtIndex:1] state], NSOnState); + EXPECT_EQ(2, [menu numberOfItems]); + EXPECT_EQ(NSOnState, [[menu itemAtIndex:1] state]); // Close the popup and verify it's no longer in the view hierarchy. This // means HideView() has been called. diff --git a/chrome/browser/gtk/blocked_popup_container_view_gtk.cc b/chrome/browser/gtk/blocked_popup_container_view_gtk.cc index 24016a0..fa3979e 100644 --- a/chrome/browser/gtk/blocked_popup_container_view_gtk.cc +++ b/chrome/browser/gtk/blocked_popup_container_view_gtk.cc @@ -69,7 +69,8 @@ void BlockedPopupContainerViewGtk::ShowView() { } void BlockedPopupContainerViewGtk::UpdateLabel() { - size_t blocked_popups = model_->GetBlockedPopupCount(); + size_t blocked_notices = model_->GetBlockedNoticeCount(); + size_t blocked_items = model_->GetBlockedPopupCount() + blocked_notices; GtkWidget* label = gtk_bin_get_child(GTK_BIN(menu_button_)); if (!label) { @@ -77,12 +78,17 @@ void BlockedPopupContainerViewGtk::UpdateLabel() { gtk_container_add(GTK_CONTAINER(menu_button_), label); } - gtk_label_set_text( - GTK_LABEL(label), - (blocked_popups > 0) ? - l10n_util::GetStringFUTF8(IDS_POPUPS_BLOCKED_COUNT, - UintToString16(blocked_popups)).c_str() : - l10n_util::GetStringUTF8(IDS_POPUPS_UNBLOCKED).c_str()); + std::string label_text; + if (blocked_items == 0) { + label_text = l10n_util::GetStringUTF8(IDS_POPUPS_UNBLOCKED); + } else if (blocked_notices == 0) { + label_text = l10n_util::GetStringFUTF8(IDS_POPUPS_BLOCKED_COUNT, + UintToString16(blocked_items)); + } else { + label_text = l10n_util::GetStringFUTF8(IDS_BLOCKED_NOTICE_COUNT, + UintToString16(blocked_items)); + } + gtk_label_set_text(GTK_LABEL(label), label_text.c_str()); } void BlockedPopupContainerViewGtk::HideView() { @@ -122,11 +128,14 @@ bool BlockedPopupContainerViewGtk::IsCommandEnabled(int command_id) const { } bool BlockedPopupContainerViewGtk::IsItemChecked(int id) const { + // |id| should be > 0 since all index based commands have 1 added to them. DCHECK_GT(id, 0); size_t id_size_t = static_cast<size_t>(id); + if (id_size_t > BlockedPopupContainer::kImpossibleNumberOfPopups) { - return model_->IsHostWhitelisted( - id_size_t - BlockedPopupContainer::kImpossibleNumberOfPopups - 1); + id_size_t -= BlockedPopupContainer::kImpossibleNumberOfPopups + 1; + if (id_size_t < model_->GetPopupHostCount()) + return model_->IsHostWhitelisted(id_size_t); } return false; @@ -135,14 +144,31 @@ bool BlockedPopupContainerViewGtk::IsItemChecked(int id) const { void BlockedPopupContainerViewGtk::ExecuteCommand(int id) { DCHECK_GT(id, 0); size_t id_size_t = static_cast<size_t>(id); - if (id_size_t > BlockedPopupContainer::kImpossibleNumberOfPopups) { - // Decrement id since all index based commands have 1 added to them. (See - // ButtonPressed() for detail). - model_->ToggleWhitelistingForHost( - id_size_t - BlockedPopupContainer::kImpossibleNumberOfPopups - 1); - } else { + + // Is this a click on a popup? + if (id_size_t < BlockedPopupContainer::kImpossibleNumberOfPopups) { model_->LaunchPopupAtIndex(id_size_t - 1); + return; + } + + // |id| shouldn't be == kImpossibleNumberOfPopups since the popups end before + // this and the hosts start after it. (If it is used, it is as a separator.) + DCHECK_NE(id_size_t, BlockedPopupContainer::kImpossibleNumberOfPopups); + id_size_t -= BlockedPopupContainer::kImpossibleNumberOfPopups + 1; + + // Is this a click on a host? + size_t host_count = model_->GetPopupHostCount(); + if (id_size_t < host_count) { + model_->ToggleWhitelistingForHost(id_size_t); + return; } + + // |id shouldn't be == host_count since this is the separator between hosts + // and notices. + DCHECK_NE(id_size_t, host_count); + id_size_t -= host_count + 1; + + // Nothing to do for now for notices. } BlockedPopupContainerViewGtk::BlockedPopupContainerViewGtk( @@ -200,17 +226,32 @@ void BlockedPopupContainerViewGtk::OnMenuButtonClicked( } // Set items (kImpossibleNumberOfPopups + 1) .. - // (kImpossibleNumberOfPopups + 1 + hosts.size()) as hosts. + // (kImpossibleNumberOfPopups + hosts.size()) as hosts. std::vector<std::string> hosts(container->model_->GetHosts()); if (!hosts.empty() && (popup_count > 0)) container->launch_menu_->AppendSeparator(); + size_t first_host = BlockedPopupContainer::kImpossibleNumberOfPopups + 1; for (size_t i = 0; i < hosts.size(); ++i) { - container->launch_menu_->AppendCheckMenuItemWithLabel( - BlockedPopupContainer::kImpossibleNumberOfPopups + i + 1, + container->launch_menu_->AppendCheckMenuItemWithLabel(first_host + i, l10n_util::GetStringFUTF8(IDS_POPUP_HOST_FORMAT, UTF8ToUTF16(hosts[i]))); } + // Set items (kImpossibleNumberOfPopups + hosts.size() + 2) .. + // (kImpossibleNumberOfPopups + hosts.size() + 1 + notice_count) as notices. + size_t notice_count = container->model_->GetBlockedNoticeCount(); + if (notice_count && (!hosts.empty() || (popup_count > 0))) + container->launch_menu_->AppendSeparator(); + size_t first_notice = first_host + hosts.size() + 1; + for (size_t i = 0; i < notice_count; ++i) { + std::string host; + string16 reason; + container->model_->GetHostAndReasonForNotice(i, &host, &reason); + container->launch_menu_->AppendMenuItemWithLabel(first_notice + i, + l10n_util::GetStringFUTF8(IDS_NOTICE_TITLE_FORMAT, UTF8ToUTF16(host), + reason)); + } + container->launch_menu_->PopupAsContext(gtk_get_current_event_time()); } diff --git a/chrome/browser/views/blocked_popup_container_view_win.cc b/chrome/browser/views/blocked_popup_container_view_win.cc index 85fba6b..00e4157 100644 --- a/chrome/browser/views/blocked_popup_container_view_win.cc +++ b/chrome/browser/views/blocked_popup_container_view_win.cc @@ -27,11 +27,6 @@ namespace { // The minimal border around the edge of the notification. const int kSmallPadding = 2; -// The offset needed for blocked notices not to clash with anything else. -// Basically 2 separators (one between popups and hosts, one between hosts -// and notices). -const int kNoticeMenuOffset = 2; - // The background color of the blocked popup notification. const SkColor kBackgroundColorTop = SkColorSetRGB(255, 242, 183); const SkColor kBackgroundColorBottom = SkColorSetRGB(250, 230, 145); @@ -157,13 +152,14 @@ BlockedPopupContainerInternalView::~BlockedPopupContainerInternalView() { } void BlockedPopupContainerInternalView::UpdateLabel() { - size_t blocked_items = - container_->GetBlockedPopupCount() + container_->GetBlockedNoticeCount(); + size_t blocked_notices = container_->model()->GetBlockedNoticeCount(); + size_t blocked_items = container_->model()->GetBlockedPopupCount() + + blocked_notices; std::wstring label; if (blocked_items == 0) { label = l10n_util::GetString(IDS_POPUPS_UNBLOCKED); - } else if (container_->GetBlockedNoticeCount() == 0) { + } else if (blocked_notices == 0) { label = l10n_util::GetStringF(IDS_POPUPS_BLOCKED_COUNT, UintToWString(blocked_items)); } else { @@ -230,82 +226,105 @@ gfx::Size BlockedPopupContainerInternalView::GetPreferredSize() { void BlockedPopupContainerInternalView::ButtonPressed( views::Button* sender, const views::Event& event) { - if (sender == popup_count_label_) { - launch_menu_.reset(views::Menu::Create(this, views::Menu::TOPLEFT, - container_->GetNativeView())); - - // Set items 1 .. popup_count as individual popups. - size_t popup_count = container_->GetBlockedPopupCount(); - for (size_t i = 0; i < popup_count; ++i) { - std::wstring url, title; - container_->GetURLAndTitleForPopup(i, &url, &title); - // We can't just use the index into container_ here because Menu reserves - // the value 0 as the nop command. - launch_menu_->AppendMenuItem(i + 1, - l10n_util::GetStringF(IDS_POPUP_TITLE_FORMAT, url, title), - views::Menu::NORMAL); - } + if (sender == close_button_) { + container_->model()->set_dismissed(); + container_->model()->CloseAll(); + } - // Set items (kImpossibleNumberOfPopups + 1) .. - // (kImpossibleNumberOfPopups + 1 + hosts.size()) as hosts. - std::vector<std::wstring> hosts(container_->GetHosts()); - if (!hosts.empty() && (popup_count > 0)) - launch_menu_->AppendSeparator(); - for (size_t i = 0; i < hosts.size(); ++i) { - launch_menu_->AppendMenuItem( - BlockedPopupContainer::kImpossibleNumberOfPopups + i + 1, - l10n_util::GetStringF(IDS_POPUP_HOST_FORMAT, hosts[i]), - views::Menu::NORMAL); - } + if (sender != popup_count_label_) + return; + + launch_menu_.reset(views::Menu::Create(this, views::Menu::TOPLEFT, + container_->GetNativeView())); + + // Set items 1 .. popup_count as individual popups. + size_t popup_count = container_->model()->GetBlockedPopupCount(); + for (size_t i = 0; i < popup_count; ++i) { + std::wstring url, title; + container_->GetURLAndTitleForPopup(i, &url, &title); + // We can't just use the index into container_ here because Menu reserves + // the value 0 as the nop command. + launch_menu_->AppendMenuItem(i + 1, + l10n_util::GetStringF(IDS_POPUP_TITLE_FORMAT, url, title), + views::Menu::NORMAL); + } - size_t notice_count = container_->GetBlockedNoticeCount(); - if (notice_count) - launch_menu_->AppendSeparator(); - - for (size_t i = 0; i < notice_count; ++i) { - std::string host; - string16 reason; - container_->GetModel()->GetHostAndReasonForNotice(i, &host, &reason); - launch_menu_->AppendMenuItem( - BlockedPopupContainer::kImpossibleNumberOfPopups + - hosts.size() + i + kNoticeMenuOffset + 1, - l10n_util::GetStringF(IDS_NOTICE_TITLE_FORMAT, - ASCIIToWide(host), reason), - views::Menu::NORMAL); - } + // Set items (kImpossibleNumberOfPopups + 1) .. + // (kImpossibleNumberOfPopups + hosts.size()) as hosts. + std::vector<std::string> hosts(container_->model()->GetHosts()); + if (!hosts.empty() && (popup_count > 0)) + launch_menu_->AppendSeparator(); + size_t first_host = BlockedPopupContainer::kImpossibleNumberOfPopups + 1; + for (size_t i = 0; i < hosts.size(); ++i) { + launch_menu_->AppendMenuItem(first_host + i, + l10n_util::GetStringF(IDS_POPUP_HOST_FORMAT, UTF8ToWide(hosts[i])), + views::Menu::NORMAL); + } - CPoint cursor_position; - ::GetCursorPos(&cursor_position); - launch_menu_->RunMenuAt(cursor_position.x, cursor_position.y); - } else if (sender == close_button_) { - container_->GetModel()->set_dismissed(); - container_->GetModel()->CloseAll(); + // Set items (kImpossibleNumberOfPopups + hosts.size() + 2) .. + // (kImpossibleNumberOfPopups + hosts.size() + 1 + notice_count) as notices. + size_t notice_count = container_->model()->GetBlockedNoticeCount(); + if (notice_count && (!hosts.empty() || (popup_count > 0))) + launch_menu_->AppendSeparator(); + size_t first_notice = first_host + hosts.size() + 1; + for (size_t i = 0; i < notice_count; ++i) { + std::string host; + string16 reason; + container_->model()->GetHostAndReasonForNotice(i, &host, &reason); + launch_menu_->AppendMenuItem(first_notice + i, + l10n_util::GetStringF(IDS_NOTICE_TITLE_FORMAT, ASCIIToWide(host), + reason), + views::Menu::NORMAL); } + + CPoint cursor_position; + GetCursorPos(&cursor_position); + launch_menu_->RunMenuAt(cursor_position.x, cursor_position.y); } bool BlockedPopupContainerInternalView::IsItemChecked(int id) const { - if (id > BlockedPopupContainer::kImpossibleNumberOfPopups) { - return container_->GetModel()->IsHostWhitelisted(static_cast<size_t>( - id - BlockedPopupContainer::kImpossibleNumberOfPopups - 1)); + // |id| should be > 0 since all index based commands have 1 added to them. + DCHECK_GT(id, 0); + size_t id_size_t = static_cast<size_t>(id); + + if (id_size_t > BlockedPopupContainer::kImpossibleNumberOfPopups) { + id_size_t -= BlockedPopupContainer::kImpossibleNumberOfPopups + 1; + if (id_size_t < container_->model()->GetPopupHostCount()) + return container_->model()->IsHostWhitelisted(id_size_t); } return false; } void BlockedPopupContainerInternalView::ExecuteCommand(int id) { + // |id| should be > 0 since all index based commands have 1 added to them. DCHECK_GT(id, 0); size_t id_size_t = static_cast<size_t>(id); - if (id_size_t > BlockedPopupContainer::kImpossibleNumberOfPopups + - container_->GetModel()->GetPopupHostCount() + kNoticeMenuOffset) { - // Nothing to do for now for notices. - } else if (id_size_t > BlockedPopupContainer::kImpossibleNumberOfPopups) { - // Decrement id since all index based commands have 1 added to them. (See - // ButtonPressed() for detail). - container_->GetModel()->ToggleWhitelistingForHost( - id_size_t - BlockedPopupContainer::kImpossibleNumberOfPopups - 1); - } else { - container_->GetModel()->LaunchPopupAtIndex(id_size_t - 1); + + // Is this a click on a popup? + if (id_size_t < BlockedPopupContainer::kImpossibleNumberOfPopups) { + container_->model()->LaunchPopupAtIndex(id_size_t - 1); + return; } + + // |id| shouldn't be == kImpossibleNumberOfPopups since the popups end before + // this and the hosts start after it. (If it is used, it is as a separator.) + DCHECK_NE(id_size_t, BlockedPopupContainer::kImpossibleNumberOfPopups); + id_size_t -= BlockedPopupContainer::kImpossibleNumberOfPopups + 1; + + // Is this a click on a host? + size_t host_count = container_->model()->GetPopupHostCount(); + if (id_size_t < host_count) { + container_->model()->ToggleWhitelistingForHost(id_size_t); + return; + } + + // |id shouldn't be == host_count since this is the separator between hosts + // and notices. + DCHECK_NE(id_size_t, host_count); + id_size_t -= host_count + 1; + + // Nothing to do for now for notices. } @@ -323,30 +342,12 @@ void BlockedPopupContainerViewWin::GetURLAndTitleForPopup( size_t index, std::wstring* url, std::wstring* title) const { DCHECK(url); DCHECK(title); - TabContents* tab_contents = GetModel()->GetTabContentsAt(index); + TabContents* tab_contents = model()->GetTabContentsAt(index); const GURL& tab_contents_url = tab_contents->GetURL().GetOrigin(); *url = UTF8ToWide(tab_contents_url.possibly_invalid_spec()); *title = UTF16ToWideHack(tab_contents->GetTitle()); } -std::vector<std::wstring> BlockedPopupContainerViewWin::GetHosts() const { - std::vector<std::string> utf8_hosts(GetModel()->GetHosts()); - - std::vector<std::wstring> hosts; - for (std::vector<std::string>::const_iterator it = utf8_hosts.begin(); - it != utf8_hosts.end(); ++it) - hosts.push_back(UTF8ToWide(*it)); - return hosts; -} - -size_t BlockedPopupContainerViewWin::GetBlockedPopupCount() const { - return container_model_->GetBlockedPopupCount(); -} - -size_t BlockedPopupContainerViewWin::GetBlockedNoticeCount() const { - return container_model_->GetBlockedNoticeCount(); -} - // Overridden from AnimationDelegate: void BlockedPopupContainerViewWin::AnimationStarted( @@ -432,13 +433,13 @@ void BlockedPopupContainerViewWin::Destroy() { BlockedPopupContainerViewWin::BlockedPopupContainerViewWin( BlockedPopupContainer* container) : slide_animation_(new SlideAnimation(this)), - container_model_(container), + model_(container), container_view_(NULL) { container_view_ = new BlockedPopupContainerInternalView(this); container_view_->SetVisible(true); set_window_style(WS_CHILD | WS_CLIPSIBLINGS | WS_CLIPCHILDREN); - WidgetWin::Init(GetModel()->GetConstrainingContents(NULL)->GetNativeView(), + WidgetWin::Init(model_->GetConstrainingContents(NULL)->GetNativeView(), gfx::Rect()); SetContentsView(container_view_); SetPosition(); diff --git a/chrome/browser/views/blocked_popup_container_view_win.h b/chrome/browser/views/blocked_popup_container_view_win.h index 756a4de..ad27ea5 100644 --- a/chrome/browser/views/blocked_popup_container_view_win.h +++ b/chrome/browser/views/blocked_popup_container_view_win.h @@ -45,17 +45,8 @@ class BlockedPopupContainerViewWin : public BlockedPopupContainerView, std::wstring* url, std::wstring* title) const; - // Returns the names of hosts showing popups. - std::vector<std::wstring> GetHosts() const; - - // Returns the number of blocked popups from the model - size_t GetBlockedPopupCount() const; - - // Returns the number of blocked notices from the model - size_t GetBlockedNoticeCount() const; - // Returns the model that owns us. - BlockedPopupContainer* GetModel() const { return container_model_; } + BlockedPopupContainer* model() const { return model_; } // Overridden from AnimationDelegate: virtual void AnimationStarted(const Animation* animation); @@ -82,7 +73,7 @@ class BlockedPopupContainerViewWin : public BlockedPopupContainerView, virtual void OnSize(UINT param, const CSize& size); // Our model; calling the shots. - BlockedPopupContainer* container_model_; + BlockedPopupContainer* model_; // Our associated view object. BlockedPopupContainerInternalView* container_view_; |