diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-02-02 10:40:49 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-02-02 10:40:49 +0000 |
commit | 8cc8d4914c363a3308fdbae1415e2a3fbbce8e9e (patch) | |
tree | f2b38543aa09fd9945341376bfca465b7efbbf92 /chrome/browser/cocoa | |
parent | 458788376076d57d182ca22060b335d2e5f2e18f (diff) | |
download | chromium_src-8cc8d4914c363a3308fdbae1415e2a3fbbce8e9e.zip chromium_src-8cc8d4914c363a3308fdbae1415e2a3fbbce8e9e.tar.gz chromium_src-8cc8d4914c363a3308fdbae1415e2a3fbbce8e9e.tar.bz2 |
Wire popup blocking UI to the new machinery and port over the user's old whitelist.
Notably, this removes the UI entirely for Mac and Linux. We need to rebuild it.
This also guts the old system's testing, since most of it disappeared or changed radically. We should test the new stuff. I will file a followup bug for that.
There are various tiny edge cases, like if you click the address bar icon really quickly, sometimes you'll get popups without any title yet, which makes them leave gaps in the bubble that appears. We can fix that sort of thing. The critical bit I tried to ensure was that we never try to open a dead popup or use a dead TabContents, no matter what.
BUG=33314
TEST=Go visit some sites with popups and play with things
Review URL: http://codereview.chromium.org/562013
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@37819 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/cocoa')
3 files changed, 0 insertions, 545 deletions
diff --git a/chrome/browser/cocoa/blocked_popup_container_controller.h b/chrome/browser/cocoa/blocked_popup_container_controller.h deleted file mode 100644 index df3eb64..0000000 --- a/chrome/browser/cocoa/blocked_popup_container_controller.h +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) 2009 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_VIEWS_BLOCKED_POPUP_CONTAINER_CONTROLLER_H_ -#define CHROME_BROWSER_VIEWS_BLOCKED_POPUP_CONTAINER_CONTROLLER_H_ - -#import <Cocoa/Cocoa.h> - -#include "base/scoped_nsobject.h" -#include "base/scoped_ptr.h" -#include "chrome/browser/blocked_popup_container.h" -#include "chrome/browser/cocoa/hover_close_button.h" - -@class BubbleView; - -// Controller for the blocked popup view. Communicates with the cross-platform -// code via a C++ bridge class, below. The BlockedPopupContainer class doesn't -// really "own" the bridge, it just keeps a pointer to it and calls Destroy() on -// it when it's supposed to go away. As a result, this class needs to own itself -// (always keep an extra retain), and will autorelease itself (and the bridge -// which it owns) when the bridge gets a Destroy() message. -// TODO(pinkerton): Reverse the ownership if it makes more sense. I'm leaving -// it this way because I assume we eventually want this to be a -// NSViewController, and we usually have the Obj-C controller owning the -// bridge (rather than the other way around). -@interface BlockedPopupContainerController : NSObject { - @private - scoped_ptr<BlockedPopupContainerView> bridge_; - BlockedPopupContainer* container_; // Weak. "owns" me. - scoped_nsobject<BubbleView> view_; - scoped_nsobject<HoverCloseButton> closeButton_; - - IBOutlet NSPopUpButton* popupButton_; -} - -// Initialize with the given popup container. Creates the C++ bridge object -// used to represet the "view". -- (id)initWithContainer:(BlockedPopupContainer*)container; - -// Returns the C++ brige object. -- (BlockedPopupContainerView*)bridge; - -// Called by the bridge to perform certain actions from the back-end code. -- (void)show; -- (void)hide; -- (void)update; - -@end - -@interface BlockedPopupContainerController(ForTesting) -- (BubbleView*)view; -- (IBAction)closePopup:(id)sender; -- (NSMenu*)buildMenu; -- (void)setContainer:(BlockedPopupContainer*)container; -@end - -#endif // CHROME_BROWSER_VIEWS_BLOCKED_POPUP_CONTAINER_CONTROLLER_H_ diff --git a/chrome/browser/cocoa/blocked_popup_container_controller.mm b/chrome/browser/cocoa/blocked_popup_container_controller.mm deleted file mode 100644 index 5dc761d..0000000 --- a/chrome/browser/cocoa/blocked_popup_container_controller.mm +++ /dev/null @@ -1,382 +0,0 @@ -// Copyright (c) 2009 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. - -#import "chrome/browser/cocoa/blocked_popup_container_controller.h" - -#include "app/l10n_util_mac.h" -#include "base/nsimage_cache_mac.h" -#include "base/sys_string_conversions.h" -#import "chrome/browser/cocoa/bubble_view.h" -#include "chrome/browser/tab_contents/tab_contents.h" -#include "chrome/browser/tab_contents/tab_contents_view.h" -#include "grit/generated_resources.h" - -// A C++ bridge class that manages the interaction between the C++ interface -// and the Objective-C view controller that implements the popup blocker. -class BlockedPopupContainerViewBridge : public BlockedPopupContainerView { - public: - BlockedPopupContainerViewBridge(BlockedPopupContainerController* controller); - virtual ~BlockedPopupContainerViewBridge(); - - // Overrides from BlockedPopupContainerView - virtual void SetPosition(); - virtual void ShowView(); - virtual void UpdateLabel(); - virtual void HideView(); - virtual void Destroy(); - - private: - BlockedPopupContainerController* controller_; // Weak, owns us. -}; - -@interface BlockedPopupContainerController(Private) -- (void)initPopupView; -- (NSView*)containingView; -@end - -@implementation BlockedPopupContainerController - -// Initialize with the given popup container. Creates the C++ bridge object -// used to represent the "view". -- (id)initWithContainer:(BlockedPopupContainer*)container { - if ((self = [super init])) { - container_ = container; - bridge_.reset(new BlockedPopupContainerViewBridge(self)); - [self initPopupView]; - } - return self; -} - -- (void)dealloc { - [view_ removeFromSuperview]; - [[NSNotificationCenter defaultCenter] removeObserver:self]; - [super dealloc]; -} - -- (IBAction)closePopup:(id)sender { - container_->set_dismissed(); - container_->CloseAll(); -} - -// Create and initialize the popup view and its label, close box, etc. -- (void)initPopupView { - static const float kWidth = 200.0; - static const float kHeight = 20.0; - static const float kCloseBoxSize = 16.0; - static const float kCloseBoxPaddingY = 2.0; - static const float kLabelPaddingX = 5.0; - - NSWindow* window = [[self containingView] window]; - - // Create it below the parent's bottom edge so we can animate it into place. - NSRect startFrame = NSMakeRect(0.0, -kHeight, kWidth, kHeight); - view_.reset([[BubbleView alloc] initWithFrame:startFrame - themeProvider:window]); - [view_ setAutoresizingMask:NSViewMinXMargin | NSViewMaxYMargin]; - [view_ setCornerFlags:kRoundedTopLeftCorner | kRoundedTopRightCorner]; - - // Create the text label and position it. We'll resize it later when the - // label gets updated. The view owns the label, we only hold a weak reference. - NSRect labelFrame = NSMakeRect(kLabelPaddingX, - 0, - startFrame.size.width - kCloseBoxSize, - startFrame.size.height); - popupButton_ = [[[NSPopUpButton alloc] initWithFrame:labelFrame] autorelease]; - [popupButton_ setAutoresizingMask:NSViewWidthSizable]; - [popupButton_ setBordered:NO]; - [popupButton_ setBezelStyle:NSTexturedRoundedBezelStyle]; - [popupButton_ setPullsDown:YES]; - // TODO(pinkerton): this doesn't work, not sure why. - [popupButton_ setPreferredEdge:NSMaxYEdge]; - // TODO(pinkerton): no matter what, the arrows always draw in the middle - // of the button. We can turn off the arrows entirely, but then will the - // user ever know to click it? - [[popupButton_ cell] setArrowPosition:NSPopUpNoArrow]; - [[popupButton_ cell] setAltersStateOfSelectedItem:NO]; - // If we don't add this, no title will ever display. - [popupButton_ addItemWithTitle:@""]; - [view_ addSubview:popupButton_]; - - // Register for notifications that the menu is about to display so we can - // fill it in lazily - [[NSNotificationCenter defaultCenter] - addObserver:self - selector:@selector(showMenu:) - name:NSPopUpButtonCellWillPopUpNotification - object:[popupButton_ cell]]; - - // Create the close box and position at the left of the view. - NSRect closeFrame = NSMakeRect(startFrame.size.width - kCloseBoxSize, - kCloseBoxPaddingY, - kCloseBoxSize, - kCloseBoxSize); - closeButton_.reset([[HoverCloseButton alloc] initWithFrame:closeFrame]); - [closeButton_ setAutoresizingMask:NSViewMinXMargin]; - [closeButton_ setButtonType:NSMomentaryChangeButton]; - [closeButton_ setBordered:NO]; - [closeButton_ setTarget:self]; - [closeButton_ setAction:@selector(closePopup:)]; - [view_ addSubview:closeButton_]; -} - -// Returns the C++ brige object. -- (BlockedPopupContainerView*)bridge { - return bridge_.get(); -} - -// Returns the parent of the RWHVMac. We insert our popup blocker as a sibling -// so that it stays around as the RWHVMac is created/destroyed during -// navigation. -- (NSView*)containingView { - NSView* view = nil; - if (container_) - view = container_->GetConstrainingContents(NULL)->view()->GetNativeView(); - return view; -} - -- (void)show { - const float kLeftPadding = 20; // Leave room for the scrollbar. - - // No need to do anything if it's already on screen. - if ([view_ superview]) return; - - // Position the view at the bottom right corner, always leaving room for the - // scrollbar. This is what window does. It also doesn't care about covering - // up the horizontal scrollbar. - NSView* parent = [self containingView]; - NSRect frame = [view_ frame]; - frame.origin.x = [parent frame].size.width - frame.size.width - kLeftPadding; - [view_ setFrame:frame]; - - // Add the view and animate it sliding up into view. - [parent addSubview:view_.get()]; - frame.origin.y = 0; - [[view_ animator] setFrame:frame]; -} - -- (void)hide { - [view_ removeFromSuperview]; -} - -// Resize the view based on the new label contents. The autoresize mask will -// take care of resizing everything else. -- (void)resizeWithLabel:(NSString*)label { - // It would be nice to teach BubbleView to honor -sizeToFit, but it can't - // really handle the subviews that get added. So just measure the string - // and pad for the views. - NSDictionary* attributes = - [NSDictionary dictionaryWithObjectsAndKeys: - NSFontAttributeName, [view_ font], - nil]; - NSSize stringSize = [label sizeWithAttributes:attributes]; - // Keep the right edge in the same place. - NSRect frame = [view_ frame]; - CGFloat originalWidth = frame.size.width; - frame.size.width = - stringSize.width + [closeButton_ frame].size.width + - [popupButton_ frame].origin.x + kBubbleViewTextPositionX; - frame.origin.x -= frame.size.width - originalWidth; - [view_ setFrame:frame]; -} - -- (void)update { - size_t blockedNotices = container_->GetBlockedNoticeCount(); - size_t blockedItems = container_->GetBlockedPopupCount() + blockedNotices; - NSString* label = nil; - if (!blockedItems) { - label = l10n_util::GetNSString(IDS_POPUPS_UNBLOCKED); - } else if (!blockedNotices) { - label = l10n_util::GetNSStringF(IDS_POPUPS_BLOCKED_COUNT, - UintToString16(blockedItems)); - } else { - label = l10n_util::GetNSStringF(IDS_BLOCKED_NOTICE_COUNT, - UintToString16(blockedItems)); - } - [self resizeWithLabel:label]; - [view_ setContent:label]; -} - -// 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. 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); - 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 { -void GetURLAndTitleForPopup( - const BlockedPopupContainer* container, - size_t index, - string16* url, - string16* title) { - DCHECK(url); - DCHECK(title); - TabContents* tab_contents = container->GetTabContentsAt(index); - const GURL& tab_contents_url = tab_contents->GetURL().GetOrigin(); - *url = UTF8ToUTF16(tab_contents_url.possibly_invalid_spec()); - *title = tab_contents->GetTitle(); -} -} // namespace - -// Build a new popup menu from scratch. The menu contains the blocked popups -// (tags being the popup's index), followed by the list of hosts from which -// the popups were blocked (tags being |kImpossibleNumberOfPopups| + host -// index). The hosts are used to toggle whitelisting for a site. -- (NSMenu*)buildMenu { - NSMenu* menu = [[[NSMenu alloc] init] autorelease]; - - // For pop-down menus, the first item is what is displayed while tracking the - // menu and it remains there if nothing is selected. Set it to the - // current title. - NSString* currentTitle = [popupButton_ title]; - scoped_nsobject<NSMenuItem> dummy( - [[NSMenuItem alloc] initWithTitle:currentTitle - action:nil - keyEquivalent:@""]); - [menu addItem:dummy.get()]; - - // Add the list of blocked popups titles to the menu. We set the array index - // as the tag for use in the menu action rather than relying on the menu item - // index. - const size_t count = container_->GetBlockedPopupCount(); - for (size_t i = 0; i < count; ++i) { - string16 url, title; - GetURLAndTitleForPopup(container_, i, &url, &title); - NSString* titleStr = - l10n_util::GetNSStringF(IDS_POPUP_TITLE_FORMAT, url, title); - scoped_nsobject<NSMenuItem> item( - [[NSMenuItem alloc] initWithTitle:titleStr - action:@selector(menuAction:) - keyEquivalent:@""]); - [item setTag:i]; - [item setTarget:self]; - [menu addItem:item.get()]; - } - - // Add the list of hosts. We begin tagging these at - // |kImpossibleNumberOfPopups|. If whitelisting has already been enabled - // for a site, mark it with a checkmark. - 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])); - scoped_nsobject<NSMenuItem> item( - [[NSMenuItem alloc] initWithTitle:titleStr - action:@selector(menuAction:) - keyEquivalent:@""]); - if (container_->IsHostWhitelisted(i)) - [item setState:NSOnState]; - [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), - reason); - scoped_nsobject<NSMenuItem> item( - [[NSMenuItem alloc] initWithTitle:titleStr - action:@selector(menuAction:) - keyEquivalent:@""]); - [item setTag:first_notice + i]; - [item setTarget:self]; - [menu addItem:item.get()]; - } - - return menu; -} - -// Called when the popup button is about to display the menu, giving us a -// chance to fill in the contents. -- (void)showMenu:(NSNotification*)notify { - NSMenu* menu = [self buildMenu]; - [[notify object] setMenu:menu]; -} - -// Only used for testing. -- (BubbleView*)view { - return view_.get(); -} - -// Only used for testing. -- (void)setContainer:(BlockedPopupContainer*)container { - container_ = container; -} - -@end - -//--------------------------------------------------------------------------- - -BlockedPopupContainerView* BlockedPopupContainerView::Create( - BlockedPopupContainer* container) { - // We "leak" |blocker| for now, we'll release it when the bridge class - // gets a Destroy() message. - BlockedPopupContainerController* blocker = - [[BlockedPopupContainerController alloc] initWithContainer:container]; - return [blocker bridge]; -} - -BlockedPopupContainerViewBridge::BlockedPopupContainerViewBridge( - BlockedPopupContainerController* controller) { - controller_ = controller; -} - -BlockedPopupContainerViewBridge::~BlockedPopupContainerViewBridge() { -} - -void BlockedPopupContainerViewBridge::SetPosition() { - // Doesn't ever get called, also a no-op on GTK. - NOTIMPLEMENTED(); -} - -void BlockedPopupContainerViewBridge::ShowView() { - [controller_ show]; -} - -void BlockedPopupContainerViewBridge::UpdateLabel() { - [controller_ update]; -} - -void BlockedPopupContainerViewBridge::HideView() { - [controller_ hide]; -} - -void BlockedPopupContainerViewBridge::Destroy() { - [controller_ autorelease]; -} diff --git a/chrome/browser/cocoa/blocked_popup_container_controller_unittest.mm b/chrome/browser/cocoa/blocked_popup_container_controller_unittest.mm deleted file mode 100644 index 7cb8615..0000000 --- a/chrome/browser/cocoa/blocked_popup_container_controller_unittest.mm +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright (c) 2009 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. - -#import <Cocoa/Cocoa.h> - -#include "app/app_paths.h" -#include "base/path_service.h" -#include "base/scoped_nsautorelease_pool.h" -#import "chrome/browser/cocoa/blocked_popup_container_controller.h" -#include "chrome/browser/cocoa/browser_test_helper.h" -#include "chrome/browser/cocoa/cocoa_test_helper.h" -#include "chrome/browser/renderer_host/test/test_render_view_host.h" -#include "net/base/net_util.h" -#include "testing/gtest/include/gtest/gtest.h" - - -namespace { -const std::string host1 = "host1"; -} // namespace - -class BlockedPopupContainerControllerTest : public RenderViewHostTestHarness { - public: - virtual void SetUp() { - // This is all a bit convoluted because the standard factory Create() call - // doesn't give us access to the cocoa controller for testing (since it's - // an internal implementation detail). As a result, we need to create one - // separately and inject the bridge with a test-only Create() call. - // Unfortunate, but no way around it. - RenderViewHostTestHarness::SetUp(); - CocoaTest::BootstrapCocoa(); - cocoa_controller_ = [[BlockedPopupContainerController alloc] - initWithContainer:nil]; - EXPECT_TRUE([cocoa_controller_ bridge]); - container_ = BlockedPopupContainer::Create(contents(), profile(), - [cocoa_controller_ bridge]); - [cocoa_controller_ setContainer:container_]; - contents_->set_blocked_popup_container(container_); - } - - virtual void TearDown() { - // This will also signal the Cocoa controller to delete itself with a - // Destroy() mesage to the bridge. It also clears out the association with - // |contents_|. - container_->Destroy(); - RenderViewHostTestHarness::TearDown(); - } - - TabContents* BuildTabContents() { - // This will be deleted when the TabContents goes away. - SiteInstance* instance = SiteInstance::CreateSiteInstance(profile_.get()); - - // Set up and use TestTabContents here. - return new TestTabContents(profile_.get(), instance); - } - - GURL GetTestCase(const std::string& file) { - FilePath filename; - PathService::Get(app::DIR_TEST_DATA, &filename); - filename = filename.AppendASCII("constrained_files"); - filename = filename.AppendASCII(file); - return net::FilePathToFileURL(filename); - } - - base::ScopedNSAutoreleasePool pool; - BlockedPopupContainer* container_; - BlockedPopupContainerController* cocoa_controller_; -}; - -TEST_F(BlockedPopupContainerControllerTest, BasicPopupBlock) { - // This is taken from the popup blocker unit test. - TabContents* popup = BuildTabContents(); - popup->controller().LoadURL(GetTestCase("error"), GURL(), - PageTransition::LINK); - container_->AddTabContents(popup, gfx::Rect(), host1); - 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() - // has been called on the bridge. If the bubble has a string, then - // UpdateLabel() has been called. - EXPECT_TRUE([cocoa_controller_ view]); - EXPECT_TRUE([[cocoa_controller_ view] superview]); - 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(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(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. - [cocoa_controller_ closePopup:nil]; - EXPECT_FALSE([[cocoa_controller_ view] superview]); -} |