diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-05-08 19:38:31 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-05-08 19:38:31 +0000 |
commit | b6c874589ef2f9212314ef9d2a5f1cfc19046fe7 (patch) | |
tree | 1e42e52695c5ce65654180e296afb56294b9e99d | |
parent | 24df63781910c204fa57ac06406c0883dacc366d (diff) | |
download | chromium_src-b6c874589ef2f9212314ef9d2a5f1cfc19046fe7.zip chromium_src-b6c874589ef2f9212314ef9d2a5f1cfc19046fe7.tar.gz chromium_src-b6c874589ef2f9212314ef9d2a5f1cfc19046fe7.tar.bz2 |
Various popup UI fixes:
* Remove "show popup notification" option, pref, and all associated machinery.
* Toggling whitelisting on for a site no longer hides the "manage" button.
* Toggling whitelisting off for a site re-blocks (not closes) its popups, and does not hide the "manage" button.
Also rips the whitelist hooks out of TabContents in preparation for getting the whitelist values directly from the BlockedPopupContainer, since there was no reason to plumb everything through TabContents.
BUG=11440
Review URL: http://codereview.chromium.org/115112
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@15670 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/app/generated_resources.grd | 9 | ||||
-rw-r--r-- | chrome/browser/browser.cc | 12 | ||||
-rw-r--r-- | chrome/browser/browser.h | 1 | ||||
-rw-r--r-- | chrome/browser/download/download_request_manager.cc | 7 | ||||
-rw-r--r-- | chrome/browser/renderer_host/browser_render_process_host.cc | 20 | ||||
-rw-r--r-- | chrome/browser/renderer_host/render_widget_helper.cc | 9 | ||||
-rw-r--r-- | chrome/browser/renderer_host/render_widget_helper.h | 6 | ||||
-rw-r--r-- | chrome/browser/tab_contents/tab_contents.cc | 53 | ||||
-rw-r--r-- | chrome/browser/tab_contents/tab_contents.h | 13 | ||||
-rw-r--r-- | chrome/browser/tab_contents/tab_contents_delegate.h | 11 | ||||
-rw-r--r-- | chrome/browser/views/blocked_popup_container.cc | 354 | ||||
-rw-r--r-- | chrome/browser/views/blocked_popup_container.h | 116 | ||||
-rw-r--r-- | chrome/browser/views/options/advanced_contents_view.cc | 43 | ||||
-rw-r--r-- | chrome/browser/views/options/advanced_page_view.cc | 1 | ||||
-rw-r--r-- | chrome/common/pref_names.cc | 4 | ||||
-rw-r--r-- | chrome/common/pref_names.h | 1 |
16 files changed, 308 insertions, 352 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 683e65a..c192afb 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -3069,10 +3069,6 @@ each locale. --> Change Google Gears settings </message> - <message name="IDS_OPTIONS_SHOWPOPUPBLOCKEDNOTIFICATION" desc="The label of the checkbox that toggles the 'Notify me when a popup is blocked' option"> - Notify me when a pop-up is blocked - </message> - <message name="IDS_OPTIONS_CERTIFICATES_LABEL" desc="The info label for the 'Manage certificates' button"> Select trusted SSL certificates. </message> @@ -3710,9 +3706,12 @@ each locale. --> <message name="IDS_BLOCKED_POPUP" desc="Text on a blocked popup's window titlebar."> Blocked Pop-up </message> - <message name="IDS_POPUPS_BLOCKED_COUNT" desc="Message on the blocked popup window"> + <message name="IDS_POPUPS_BLOCKED_COUNT" desc="Message on the blocked popup window when there are blocked popups"> Pop-ups Blocked: <ph name="COUNT">$1<ex>2</ex></ph> </message> + <message name="IDS_POPUPS_UNBLOCKED" desc="Message on the blocked popup window when there are only whitelisted popups"> + Manage pop-ups + </message> <message name="IDS_POPUP_TITLE_FORMAT" desc="Order of URL - Title on the popup"> <ph name="URL">$1</ph> - <ph name="WINDOW_TITLE">$2</ph> </message> diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 7d521b6..58ab731 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -1855,6 +1855,12 @@ void Browser::MoveContents(TabContents* source, const gfx::Rect& pos) { window_->SetBounds(pos); } +void Browser::DetachContents(TabContents* source) { + int index = tabstrip_model_.GetIndexOfTabContents(source); + if (index >= 0) + tabstrip_model_.DetachTabContentsAt(index); +} + bool Browser::IsPopup(TabContents* source) { // A non-tabbed BROWSER is an unconstrained popup. return (type() & TYPE_POPUP); @@ -1904,15 +1910,11 @@ bool Browser::IsApplication() const { } void Browser::ConvertContentsToApplication(TabContents* contents) { - int index = tabstrip_model_.GetIndexOfTabContents(contents); - if (index < 0) - return; - const GURL& url = contents->controller().GetActiveEntry()->url(); std::wstring app_name = ComputeApplicationNameFromURL(url); RegisterAppPrefs(app_name); - tabstrip_model_.DetachTabContentsAt(index); + DetachContents(contents); Browser* browser = Browser::CreateForApp(app_name, profile_, false); browser->tabstrip_model()->AppendTabContents(contents, true); browser->window()->Show(); diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index a2702a7..5b4db92 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -470,6 +470,7 @@ class Browser : public TabStripModelDelegate, virtual void LoadingStateChanged(TabContents* source); virtual void CloseContents(TabContents* source); virtual void MoveContents(TabContents* source, const gfx::Rect& pos); + virtual void DetachContents(TabContents* source); virtual bool IsPopup(TabContents* source); virtual void ToolbarSizeChanged(TabContents* source, bool is_animating); virtual void URLStarredChanged(TabContents* source, bool starred); diff --git a/chrome/browser/download/download_request_manager.cc b/chrome/browser/download/download_request_manager.cc index 4cd4319..4364700 100644 --- a/chrome/browser/download/download_request_manager.cc +++ b/chrome/browser/download/download_request_manager.cc @@ -241,11 +241,10 @@ void DownloadRequestManager::CanDownload(int render_process_host_id, void DownloadRequestManager::CanDownloadImpl( TabContents* originating_tab, Callback* callback) { + // If the tab requesting the download is a constrained popup that is not + // shown, treat the request as if it came from the parent. TabContents* effective_tab = originating_tab; - if (effective_tab->delegate() && - effective_tab->delegate()->GetConstrainingContents(effective_tab)) { - // The tab requesting the download is a constrained popup that is not - // shown, treat the request as if it came from the parent. + if (effective_tab->delegate()) { effective_tab = effective_tab->delegate()->GetConstrainingContents(effective_tab); } diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc index 5986fe9..0517ee3 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.cc +++ b/chrome/browser/renderer_host/browser_render_process_host.cc @@ -39,8 +39,6 @@ #include "chrome/common/child_process_info.h" #include "chrome/common/logging_chrome.h" #include "chrome/common/notification_service.h" -#include "chrome/common/pref_names.h" -#include "chrome/common/pref_service.h" #include "chrome/common/process_watcher.h" #include "chrome/common/render_messages.h" #include "chrome/common/result_codes.h" @@ -131,11 +129,6 @@ BrowserRenderProcessHost::BrowserRenderProcessHost(Profile* profile) this, &BrowserRenderProcessHost::ClearTransportDIBCache)) { widget_helper_ = new RenderWidgetHelper(); - PrefService* prefs = profile->GetPrefs(); - prefs->AddPrefObserver(prefs::kBlockPopups, this); - widget_helper_->set_block_popups( - profile->GetPrefs()->GetBoolean(prefs::kBlockPopups)); - NotificationService::current()->AddObserver(this, NotificationType::USER_SCRIPTS_LOADED, NotificationService::AllSources()); @@ -173,8 +166,6 @@ BrowserRenderProcessHost::~BrowserRenderProcessHost() { ProcessWatcher::EnsureProcessTerminated(process_.handle()); } - profile()->GetPrefs()->RemovePrefObserver(prefs::kBlockPopups, this); - NotificationService::current()->RemoveObserver(this, NotificationType::USER_SCRIPTS_LOADED, NotificationService::AllSources()); @@ -779,17 +770,6 @@ void BrowserRenderProcessHost::Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details) { switch (type.value) { - case NotificationType::PREF_CHANGED: { - std::wstring* pref_name_in = Details<std::wstring>(details).ptr(); - DCHECK(Source<PrefService>(source).ptr() == profile()->GetPrefs()); - if (*pref_name_in == prefs::kBlockPopups) { - widget_helper_->set_block_popups( - profile()->GetPrefs()->GetBoolean(prefs::kBlockPopups)); - } else { - NOTREACHED() << "unexpected pref change notification" << *pref_name_in; - } - break; - } case NotificationType::USER_SCRIPTS_LOADED: { base::SharedMemory* shared_memory = Details<base::SharedMemory>(details).ptr(); diff --git a/chrome/browser/renderer_host/render_widget_helper.cc b/chrome/browser/renderer_host/render_widget_helper.cc index d738a75..7105f8c 100644 --- a/chrome/browser/renderer_host/render_widget_helper.cc +++ b/chrome/browser/renderer_host/render_widget_helper.cc @@ -51,7 +51,6 @@ RenderWidgetHelper::RenderWidgetHelper() #elif defined(OS_POSIX) event_(false /* auto-reset */, false), #endif - block_popups_(false), resource_dispatcher_host_(NULL) { } @@ -208,14 +207,6 @@ void RenderWidgetHelper::CreateNewWindow(int opener_id, base::ProcessHandle render_process, int* route_id, ModalDialogEvent* modal_dialog_event) { - if (!user_gesture && block_popups_) { - *route_id = MSG_ROUTING_NONE; -#if defined(OS_WIN) - modal_dialog_event->event = NULL; -#endif - return; - } - *route_id = GetNextRoutingID(); ModalDialogEvent modal_dialog_event_internal; diff --git a/chrome/browser/renderer_host/render_widget_helper.h b/chrome/browser/renderer_host/render_widget_helper.h index 0e1641b..48f6087 100644 --- a/chrome/browser/renderer_host/render_widget_helper.h +++ b/chrome/browser/renderer_host/render_widget_helper.h @@ -95,9 +95,6 @@ class RenderWidgetHelper : // Gets the next available routing id. This is thread safe. int GetNextRoutingID(); - // Sets whether popup blocking is enabled or not. - void set_block_popups(bool block) { block_popups_ = block; } - // UI THREAD ONLY ----------------------------------------------------------- @@ -197,9 +194,6 @@ class RenderWidgetHelper : // The next routing id to use. base::AtomicSequenceNumber next_routing_id_; - // Whether popup blocking is enabled or not. - bool block_popups_; - ResourceDispatcherHost* resource_dispatcher_host_; DISALLOW_COPY_AND_ASSIGN(RenderWidgetHelper); diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index e932939..9cea219 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -389,8 +389,6 @@ void TabContents::RegisterUserPrefs(PrefService* prefs) { IDS_USES_UNIVERSAL_DETECTOR); prefs->RegisterLocalizedStringPref(prefs::kStaticEncodings, IDS_STATIC_ENCODING_LIST); - - prefs->RegisterBooleanPref(prefs::kBlockPopups, false); } bool TabContents::SupportsURL(GURL* url) { @@ -799,27 +797,16 @@ void TabContents::AddNewContents(TabContents* new_contents, !CommandLine::ForCurrentProcess()->HasSwitch( switches::kDisablePopupBlocking)) { // Unrequested popups from normal pages are constrained unless they're in - // the whitelist. - std::string host; - if (creator_url.is_valid()) - host = creator_url.host(); - constrain_popup = true; // TODO(pkasting): Add whitelist - - TabContents* our_owner = delegate_->GetConstrainingContents(this); - TabContents* popup_owner = our_owner ? our_owner : this; - if (constrain_popup) - popup_owner->AddConstrainedPopup(new_contents, initial_pos, host); - else - popup_owner->OnPopupOpenedFromWhitelistedHost(host); - } - if (!constrain_popup) { + // the whitelist. The popup owner will handle checking this. + delegate_->GetConstrainingContents(this)->AddPopup(new_contents, + initial_pos, + creator_url.is_valid() ? creator_url.host() : std::string()); + } else { new_contents->DisassociateFromPopupCount(); - delegate_->AddNewContents(this, new_contents, disposition, initial_pos, user_gesture); - - PopupNotificationVisibilityChanged(ShowingBlockedPopupNotification()); } + PopupNotificationVisibilityChanged(ShowingBlockedPopupNotification()); #else // TODO(port): implement the popup blocker stuff delegate_->AddNewContents(this, new_contents, disposition, initial_pos, @@ -959,15 +946,9 @@ void TabContents::ToolbarSizeChanged(bool is_animating) { void TabContents::OnStartDownload(DownloadItem* download) { DCHECK(download); - TabContents* tab_contents = this; -// TODO(port): port contraining contents. -#if defined(OS_WIN) // Download in a constrained popup is shown in the tab that opened it. - TabContents* constraining_tab = delegate()->GetConstrainingContents(this); - if (constraining_tab) - tab_contents = constraining_tab; -#endif + TabContents* tab_contents = delegate()->GetConstrainingContents(this); // GetDownloadShelf creates the download shelf if it was not yet created. tab_contents->GetDownloadShelf()->AddDownload( @@ -1135,12 +1116,6 @@ bool TabContents::IsActiveEntry(int32 page_id) { active_entry->page_id() == page_id); } -#if defined(OS_WIN) -void TabContents::SetWhitelistForHost(const std::string& host, bool whitelist) { - // TODO(pkasting): Add whitelist -} -#endif - // Notifies the RenderWidgetHost instance about the fact that the page is // loading, or done loading and calls the base implementation. void TabContents::SetIsLoading(bool is_loading, @@ -1187,17 +1162,11 @@ void TabContents::CreateBlockedPopupContainerIfNecessary() { child_windows_.push_back(blocked_popups_); } -void TabContents::AddConstrainedPopup(TabContents* new_contents, - const gfx::Rect& initial_pos, - const std::string& host) { +void TabContents::AddPopup(TabContents* new_contents, + const gfx::Rect& initial_pos, + const std::string& host) { CreateBlockedPopupContainerIfNecessary(); blocked_popups_->AddTabContents(new_contents, initial_pos, host); - PopupNotificationVisibilityChanged(ShowingBlockedPopupNotification()); -} - -void TabContents::OnPopupOpenedFromWhitelistedHost(const std::string& host) { - CreateBlockedPopupContainerIfNecessary(); - blocked_popups_->OnPopupOpenedFromWhitelistedHost(host); } // TODO(brettw) This should be on the TabContentsView. @@ -2032,7 +2001,7 @@ void TabContents::RunJavaScriptMessage( bool suppress_this_message = suppress_javascript_messages_; if (delegate()) suppress_this_message |= - (delegate()->GetConstrainingContents(this) != NULL); + (delegate()->GetConstrainingContents(this) != this); *did_suppress_message = suppress_this_message; diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index a546df5..4f938a2 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -565,9 +565,6 @@ class TabContents : public PageNavigator, render_view_host()->WindowMoveOrResizeStarted(); } - // Sets popup whitelisting state for |host| to |whitelist|. - void SetWhitelistForHost(const std::string& host, bool whitelist); - private: friend class NavigationController; // Used to access the child_windows_ (ConstrainedWindowList) for testing @@ -621,13 +618,9 @@ class TabContents : public PageNavigator, void CreateBlockedPopupContainerIfNecessary(); // Adds the incoming |new_contents| to the |blocked_popups_| container. - void AddConstrainedPopup(TabContents* new_contents, - const gfx::Rect& initial_pos, - const std::string& host); - - // Notifies the |blocked_popups_| container that a popup has been opened from - // a particular whitelisted host. - void OnPopupOpenedFromWhitelistedHost(const std::string& host); + void AddPopup(TabContents* new_contents, + const gfx::Rect& initial_pos, + const std::string& host); // Called by a derived class when the TabContents is resized, causing // suppressed constrained web popups to be repositioned to the new bounds diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h index ad7ac1e..3485fb8 100644 --- a/chrome/browser/tab_contents/tab_contents_delegate.h +++ b/chrome/browser/tab_contents/tab_contents_delegate.h @@ -64,13 +64,18 @@ class TabContentsDelegate { // in screen coordinates. virtual void MoveContents(TabContents* source, const gfx::Rect& pos) = 0; + // Causes the delegate to detach |source| and clean up any internal data + // pointing to it. After this call ownership of |source| passes to the + // caller, and it is safe to call "source->set_delegate(someone_else);". + virtual void DetachContents(TabContents* source) { } + // Called to determine if the TabContents is contained in a popup window. virtual bool IsPopup(TabContents* source) = 0; - // Returns the tab which contains the specified tab content if it is - // constrained, NULL otherwise. + // If |source| is constrained, returns the tab containing it. Otherwise + // returns |source|. virtual TabContents* GetConstrainingContents(TabContents* source) { - return NULL; + return source; } // Notification that some of our content has changed size as diff --git a/chrome/browser/views/blocked_popup_container.cc b/chrome/browser/views/blocked_popup_container.cc index abb0f35..29d1286 100644 --- a/chrome/browser/views/blocked_popup_container.cc +++ b/chrome/browser/views/blocked_popup_container.cc @@ -21,19 +21,12 @@ #include "chrome/browser/profile.h" #include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/common/notification_service.h" -#include "chrome/common/pref_names.h" #include "grit/generated_resources.h" #include "grit/theme_resources.h" #include "views/background.h" #include "views/controls/button/image_button.h" -#include "views/controls/button/menu_button.h" namespace { -// Menu item ID for the "Notify me when a popup is blocked" checkbox. (All -// other menu IDs are positive and should be base 1 indexes into the vector of -// blocked popups.) -const int kNotifyMenuItem = -1; - // A number larger than the internal popup count on the Renderer; meant for // preventing a compromised renderer from exhausting GDI memory by spawning // infinite windows. @@ -57,9 +50,10 @@ const int kShowAnimationDurationMS = 200; const int kHideAnimationDurationMS = 120; const int kFramerate = 25; -// views::MenuButton requires that the largest string be passed in to its -// constructor to reserve space. "99" should preallocate enough space for all -// numbers. +// So that the MenuButton doesn't change its size as its text changes, during +// construction we feed it the strings it will be displaying, so it can set the +// max text width to the right value. "99" should preallocate enough space for +// all numbers we'd show. const int kWidestNumber = 99; // Rounded corner radius (in pixels). @@ -95,6 +89,9 @@ BlockedPopupContainerView::BlockedPopupContainerView( l10n_util::GetStringF(IDS_POPUPS_BLOCKED_COUNT, IntToWString(kWidestNumber)), NULL, true); + // Now set the text to the other possible display string so that the button + // will update its max text width (in case this string is longer). + popup_count_label_->SetText(l10n_util::GetString(IDS_POPUPS_UNBLOCKED)); popup_count_label_->set_alignment(views::TextButton::ALIGN_CENTER); AddChildView(popup_count_label_); @@ -110,14 +107,18 @@ BlockedPopupContainerView::BlockedPopupContainerView( AddChildView(close_button_); set_background(views::Background::CreateStandardPanelBackground()); - UpdatePopupCountLabel(); + UpdateLabel(); } BlockedPopupContainerView::~BlockedPopupContainerView() { } -void BlockedPopupContainerView::UpdatePopupCountLabel() { - popup_count_label_->SetText(container_->GetWindowTitle()); +void BlockedPopupContainerView::UpdateLabel() { + size_t blocked_popups = container_->GetBlockedPopupCount(); + popup_count_label_->SetText((blocked_popups > 0) ? + l10n_util::GetStringF(IDS_POPUPS_BLOCKED_COUNT, + UintToWString(blocked_popups)) : + l10n_util::GetString(IDS_POPUPS_UNBLOCKED)); Layout(); SchedulePaint(); } @@ -180,8 +181,8 @@ void BlockedPopupContainerView::ButtonPressed(views::Button* sender) { container_->GetNativeView())); // Set items 1 .. popup_count as individual popups. - int popup_count = container_->GetBlockedPopupCount(); - for (int i = 0; i < popup_count; ++i) { + 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 @@ -201,12 +202,6 @@ void BlockedPopupContainerView::ButtonPressed(views::Button* sender) { l10n_util::GetStringF(IDS_POPUP_HOST_FORMAT, hosts[i]), Menu::NORMAL); } - launch_menu_->AppendSeparator(); - launch_menu_->AppendMenuItem( - kNotifyMenuItem, - l10n_util::GetString(IDS_OPTIONS_SHOWPOPUPBLOCKEDNOTIFICATION), - Menu::NORMAL); - CPoint cursor_position; ::GetCursorPos(&cursor_position); launch_menu_->RunMenuAt(cursor_position.x, cursor_position.y); @@ -217,24 +212,24 @@ void BlockedPopupContainerView::ButtonPressed(views::Button* sender) { } bool BlockedPopupContainerView::IsItemChecked(int id) const { - if (id == kNotifyMenuItem) - return container_->GetShowBlockedPopupNotification(); - - if (id > kImpossibleNumberOfPopups) - return container_->IsHostWhitelisted(id - kImpossibleNumberOfPopups - 1); + if (id > kImpossibleNumberOfPopups) { + return container_->IsHostWhitelisted(static_cast<size_t>( + id - kImpossibleNumberOfPopups - 1)); + } return false; } void BlockedPopupContainerView::ExecuteCommand(int id) { - if (id == kNotifyMenuItem) { - container_->ToggleBlockedPopupNotification(); - } else if (id > kImpossibleNumberOfPopups) { + DCHECK(id > 0); + size_t id_size_t = static_cast<size_t>(id); + if (id_size_t > kImpossibleNumberOfPopups) { // Decrement id since all index based commands have 1 added to them. (See // ButtonPressed() for detail). - container_->ToggleWhitelistingForHost(id - kImpossibleNumberOfPopups - 1); + container_->ToggleWhitelistingForHost( + id_size_t - kImpossibleNumberOfPopups - 1); } else { - container_->LaunchPopupIndex(id - 1); + container_->LaunchPopupAtIndex(id_size_t - 1); } } @@ -257,89 +252,90 @@ BlockedPopupContainer::BlockedPopupContainer(TabContents* owner, has_been_dismissed_(false), in_show_animation_(false), visibility_percentage_(0) { - block_popup_pref_.Init( - prefs::kBlockPopups, profile->GetPrefs(), NULL); } -void BlockedPopupContainer::ToggleBlockedPopupNotification() { - bool current = block_popup_pref_.GetValue(); - block_popup_pref_.SetValue(!current); -} - -bool BlockedPopupContainer::GetShowBlockedPopupNotification() { - return !block_popup_pref_.GetValue(); -} - -void BlockedPopupContainer::AddTabContents(TabContents* blocked_contents, +void BlockedPopupContainer::AddTabContents(TabContents* tab_contents, const gfx::Rect& bounds, const std::string& host) { + bool whitelisted = false; // TODO: Check if host is on whitelist. + + // Show whitelisted popups immediately. + if (whitelisted) + owner_->AddNewContents(tab_contents, NEW_POPUP, bounds, true, GURL()); + if (has_been_dismissed_) { - // We simply bounce this popup without notice. - delete blocked_contents; + // Don't want to show any other UI. + if (!whitelisted) + delete tab_contents; // Discard blocked popups entirely. return; } - if (blocked_popups_.size() >= kImpossibleNumberOfPopups) { - delete blocked_contents; - LOG(INFO) << "Warning: Renderer is sending more popups to us than should be" - " possible. Renderer compromised?"; - return; - } + if (whitelisted) { + // Listen for this popup's destruction, so if the user closes it manually, + // we'll know to stop caring about it. + registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED, + Source<TabContents>(tab_contents)); + + unblocked_popups_[tab_contents] = host; + } else { + if (blocked_popups_.size() >= kImpossibleNumberOfPopups) { + delete tab_contents; + LOG(INFO) << "Warning: Renderer is sending more popups to us than should " + "be possible. Renderer compromised?"; + return; + } + blocked_popups_.push_back(BlockedPopup(tab_contents, bounds, host)); - blocked_contents->set_delegate(this); - blocked_popups_.push_back(BlockedPopup(blocked_contents, bounds, host)); + tab_contents->set_delegate(this); + } - PopupHosts::iterator i(popup_hosts_.find(host)); + PopupHosts::const_iterator i(popup_hosts_.find(host)); if (i == popup_hosts_.end()) - popup_hosts_[host] = false; + popup_hosts_[host] = whitelisted; else DCHECK(!i->second); // This host was already reported as whitelisted! - container_view_->UpdatePopupCountLabel(); - ShowSelf(); -} - -void BlockedPopupContainer::OnPopupOpenedFromWhitelistedHost( - const std::string& host) { - if (has_been_dismissed_) - return; - - PopupHosts::const_iterator i(popup_hosts_.find(host)); - if (i == popup_hosts_.end()) { - popup_hosts_[host] = true; - ShowSelf(); - } else { - DCHECK(i->second); // This host was already reported as not whitelisted! + // Update UI. + container_view_->UpdateLabel(); + SetWindowPos(HWND_TOP, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW); + if (!Animation::IsAnimating() && visibility_percentage_ < 1.0) { + in_show_animation_ = true; + Animation::SetDuration(kShowAnimationDurationMS); + Animation::Start(); } + owner_->PopupNotificationVisibilityChanged(true); } -void BlockedPopupContainer::LaunchPopupIndex(int index) { - if (static_cast<size_t>(index) >= blocked_popups_.size()) +void BlockedPopupContainer::LaunchPopupAtIndex(size_t index) { + if (index >= blocked_popups_.size()) return; + // Open the popup. BlockedPopups::iterator i(blocked_popups_.begin() + index); - TabContents* contents = i->tab_contents; - gfx::Rect bounds(i->bounds); - EraseHostIfNeeded(i); - blocked_popups_.erase(i); - - contents->set_delegate(NULL); - contents->DisassociateFromPopupCount(); + TabContents* tab_contents = i->tab_contents; + tab_contents->set_delegate(NULL); + owner_->AddNewContents(tab_contents, NEW_POPUP, i->bounds, true, GURL()); - // Pass this TabContents back to our owner, forcing the window to be - // displayed since user_gesture is true. - owner_->AddNewContents(contents, NEW_POPUP, bounds, true, GURL()); + const std::string& host = i->host; + if (!host.empty()) { + // Listen for this popup's destruction, so if the user closes it manually, + // we'll know to stop caring about it. + registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED, + Source<TabContents>(tab_contents)); + + // Add the popup to the unblocked list. (Do this before the below call!) + unblocked_popups_[tab_contents] = i->host; + } - container_view_->UpdatePopupCountLabel(); - if (blocked_popups_.empty() && popup_hosts_.empty()) - HideSelf(); + // Remove the popup from the blocked list. + EraseDataForPopupAndUpdateUI(i); } -int BlockedPopupContainer::GetBlockedPopupCount() const { +size_t BlockedPopupContainer::GetBlockedPopupCount() const { return blocked_popups_.size(); } -void BlockedPopupContainer::GetURLAndTitleForPopup(int index, +void BlockedPopupContainer::GetURLAndTitleForPopup(size_t index, std::wstring* url, std::wstring* title) const { DCHECK(url); @@ -358,30 +354,48 @@ std::vector<std::wstring> BlockedPopupContainer::GetHosts() const { return hosts; } -bool BlockedPopupContainer::IsHostWhitelisted(int index) const { +bool BlockedPopupContainer::IsHostWhitelisted(size_t index) const { PopupHosts::const_iterator i(ConvertHostIndexToIterator(index)); return (i == popup_hosts_.end()) ? false : i->second; } -void BlockedPopupContainer::ToggleWhitelistingForHost(int index) { +void BlockedPopupContainer::ToggleWhitelistingForHost(size_t index) { PopupHosts::const_iterator i(ConvertHostIndexToIterator(index)); const std::string& host = i->first; bool should_whitelist = !i->second; - owner_->SetWhitelistForHost(host, should_whitelist); + popup_hosts_[host] = should_whitelist; + // TODO: Update whitelist pref. + if (should_whitelist) { - for (int j = blocked_popups_.size() - 1; j >= 0; --j) { + // Open the popups in order. + for (size_t j = 0; j < blocked_popups_.size(); ) { if (blocked_popups_[j].host == host) - LaunchPopupIndex(j); + LaunchPopupAtIndex(j); // This shifts the rest of the entries down. + else + ++j; } } else { - // TODO(pkasting): Should we have some kind of handle to the open popups, so - // we can hide them all here? - popup_hosts_.erase(host); // Can't use |i| because it's a const_iterator. - if (blocked_popups_.empty() && popup_hosts_.empty()) - HideSelf(); + for (UnblockedPopups::iterator i(unblocked_popups_.begin()); + i != unblocked_popups_.end(); ) { + TabContents* tab_contents = i->first; + TabContentsDelegate* delegate = tab_contents->delegate(); + if ((i->second == host) && delegate->IsPopup(tab_contents)) { + // Convert the popup back into a blocked popup. + delegate->DetachContents(tab_contents); + tab_contents->set_delegate(this); + + // Add the popup to the blocked list. (Do this before the below call!) + gfx::Rect bounds; + tab_contents->GetContainerBounds(&bounds); + blocked_popups_.push_back(BlockedPopup(tab_contents, bounds, host)); + + // Remove the popup from the unblocked list. + i = EraseDataForPopupAndUpdateUI(i); + } else { + ++i; + } + } } - // At this point i is invalid, since the item it points to was deleted (on - // both conditional arms). } void BlockedPopupContainer::CloseAll() { @@ -409,15 +423,6 @@ void BlockedPopupContainer::RepositionConstrainedWindowTo( SetPosition(); } -std::wstring BlockedPopupContainer::GetWindowTitle() const { - return l10n_util::GetStringF(IDS_POPUPS_BLOCKED_COUNT, - IntToWString(GetBlockedPopupCount())); -} - -const gfx::Rect& BlockedPopupContainer::GetCurrentBounds() const { - return bounds_; -} - // Overridden from TabContentsDelegate: void BlockedPopupContainer::OpenURLFromTab(TabContents* source, const GURL& url, @@ -439,17 +444,14 @@ void BlockedPopupContainer::AddNewContents(TabContents* source, void BlockedPopupContainer::CloseContents(TabContents* source) { for (BlockedPopups::iterator it = blocked_popups_.begin(); it != blocked_popups_.end(); ++it) { - if (it->tab_contents == source) { - it->tab_contents->set_delegate(NULL); - EraseHostIfNeeded(it); - blocked_popups_.erase(it); - container_view_->UpdatePopupCountLabel(); + TabContents* tab_contents = it->tab_contents; + if (tab_contents == source) { + tab_contents->set_delegate(NULL); + EraseDataForPopupAndUpdateUI(it); + delete tab_contents; break; } } - - if (blocked_popups_.empty()) - HideSelf(); } void BlockedPopupContainer::MoveContents(TabContents* source, @@ -478,13 +480,23 @@ ExtensionFunctionDispatcher* BlockedPopupContainer:: return new ExtensionFunctionDispatcher(render_view_host, NULL, extension_id); } -// Overridden from Animation: +// private: + void BlockedPopupContainer::AnimateToState(double state) { visibility_percentage_ = in_show_animation_ ? state : (1 - state); SetPosition(); } -// Override from views::WidgetWin: +void BlockedPopupContainer::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK(type == NotificationType::TAB_CONTENTS_DESTROYED); + TabContents* tab_contents = Source<TabContents>(source).ptr(); + UnblockedPopups::iterator i(unblocked_popups_.find(tab_contents)); + DCHECK(i != unblocked_popups_.end()); + EraseDataForPopupAndUpdateUI(i); +} + void BlockedPopupContainer::OnFinalMessage(HWND window) { owner_->WillClose(this); ClearData(); @@ -502,8 +514,6 @@ void BlockedPopupContainer::OnSize(UINT param, const CSize& size) { ChangeSize(param, size); } -// private: - void BlockedPopupContainer::Init(const gfx::Point& initial_anchor) { container_view_ = new BlockedPopupContainerView(this); container_view_->SetVisible(true); @@ -512,11 +522,6 @@ void BlockedPopupContainer::Init(const gfx::Point& initial_anchor) { WidgetWin::Init(owner_->GetNativeView(), gfx::Rect(), false); SetContentsView(container_view_); RepositionConstrainedWindowTo(initial_anchor); - - if (GetShowBlockedPopupNotification()) - ShowSelf(); - else - has_been_dismissed_ = true; } void BlockedPopupContainer::HideSelf() { @@ -526,16 +531,6 @@ void BlockedPopupContainer::HideSelf() { owner_->PopupNotificationVisibilityChanged(false); } -void BlockedPopupContainer::ShowSelf() { - SetWindowPos(HWND_TOP, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW); - if (!Animation::IsAnimating() && visibility_percentage_ < 1.0) { - in_show_animation_ = true; - Animation::SetDuration(kShowAnimationDurationMS); - Animation::Start(); - } - owner_->PopupNotificationVisibilityChanged(true); -} - void BlockedPopupContainer::SetPosition() { gfx::Size size = container_view_->GetPreferredSize(); int base_x = anchor_point_.x() - size.width(); @@ -564,30 +559,95 @@ void BlockedPopupContainer::ClearData() { tab_contents->set_delegate(NULL); delete tab_contents; } - blocked_popups_.clear(); + + registrar_.RemoveAll(); + unblocked_popups_.clear(); + popup_hosts_.clear(); } BlockedPopupContainer::PopupHosts::const_iterator - BlockedPopupContainer::ConvertHostIndexToIterator(int index) const { - if (static_cast<size_t>(index) >= popup_hosts_.size()) + BlockedPopupContainer::ConvertHostIndexToIterator(size_t index) const { + if (index >= popup_hosts_.size()) return popup_hosts_.end(); // If only there was a std::map::const_iterator::operator +=() ... PopupHosts::const_iterator i(popup_hosts_.begin()); - for (int j = 0; j < index; ++j) + for (size_t j = 0; j < index; ++j) ++i; return i; } -void BlockedPopupContainer::EraseHostIfNeeded(BlockedPopups::iterator i) { +void BlockedPopupContainer::EraseDataForPopupAndUpdateUI( + BlockedPopups::iterator i) { + // Erase the host if this is the last popup for that host. const std::string& host = i->host; - if (host.empty()) - return; - for (BlockedPopups::const_iterator j(blocked_popups_.begin()); - j != blocked_popups_.end(); ++j) { - if ((j != i) && (j->host == host)) - return; + if (!host.empty()) { + bool should_erase_host = true; + for (BlockedPopups::const_iterator j(blocked_popups_.begin()); + j != blocked_popups_.end(); ++j) { + if ((j != i) && (j->host == host)) { + should_erase_host = false; + break; + } + } + if (should_erase_host) { + for (UnblockedPopups::const_iterator j(unblocked_popups_.begin()); + j != unblocked_popups_.end(); ++j) { + if (j->second == host) { + should_erase_host = false; + break; + } + } + if (should_erase_host) + popup_hosts_.erase(host); + } } - popup_hosts_.erase(host); + + // Erase the popup and update the UI. + blocked_popups_.erase(i); + if (blocked_popups_.empty() && unblocked_popups_.empty()) + HideSelf(); + else + container_view_->UpdateLabel(); +} + +BlockedPopupContainer::UnblockedPopups::iterator + BlockedPopupContainer::EraseDataForPopupAndUpdateUI( + UnblockedPopups::iterator i) { + // Stop listening for this popup's destruction. + registrar_.Remove(this, NotificationType::TAB_CONTENTS_DESTROYED, + Source<TabContents>(i->first)); + + // Erase the host if this is the last popup for that host. + const std::string& host = i->second; + if (!host.empty()) { + bool should_erase_host = true; + for (UnblockedPopups::const_iterator j(unblocked_popups_.begin()); + j != unblocked_popups_.end(); ++j) { + if ((j != i) && (j->second == host)) { + should_erase_host = false; + break; + } + } + if (should_erase_host) { + for (BlockedPopups::const_iterator j(blocked_popups_.begin()); + j != blocked_popups_.end(); ++j) { + if (j->host == host) { + should_erase_host = false; + break; + } + } + if (should_erase_host) + popup_hosts_.erase(host); + } + } + + // Erase the popup and update the UI. + UnblockedPopups::iterator next_popup = unblocked_popups_.erase(i); + if (blocked_popups_.empty() && unblocked_popups_.empty()) + HideSelf(); + else + container_view_->UpdateLabel(); + return next_popup; } diff --git a/chrome/browser/views/blocked_popup_container.h b/chrome/browser/views/blocked_popup_container.h index acd0e13..894cc42 100644 --- a/chrome/browser/views/blocked_popup_container.h +++ b/chrome/browser/views/blocked_popup_container.h @@ -17,8 +17,9 @@ #include "base/gfx/rect.h" #include "chrome/browser/tab_contents/constrained_window.h" #include "chrome/browser/tab_contents/tab_contents_delegate.h" -#include "chrome/common/pref_member.h" +#include "chrome/common/notification_registrar.h" #include "views/controls/button/button.h" +#include "views/controls/button/menu_button.h" #include "views/controls/menu/menu.h" #include "views/view.h" #include "views/widget/widget_win.h" @@ -30,8 +31,6 @@ class TextButton; namespace views { class ImageButton; -class Menu; -class MenuButton; } // The view presented to the user notifying them of the number of popups @@ -44,7 +43,9 @@ class BlockedPopupContainerView : public views::View, ~BlockedPopupContainerView(); // Sets the label on the menu button - void UpdatePopupCountLabel(); + void UpdateLabel(); + + std::wstring label() const { return popup_count_label_->text(); } // Overridden from views::View: @@ -84,10 +85,11 @@ class BlockedPopupContainerView : public views::View, // Takes ownership of TabContents that are unrequested popup windows and // presents an interface to the user for launching them. (Or never showing them // again). -class BlockedPopupContainer : public ConstrainedWindow, +class BlockedPopupContainer : public Animation, + public ConstrainedWindow, + public NotificationObserver, public TabContentsDelegate, - public views::WidgetWin, - public Animation { + public views::WidgetWin { public: virtual ~BlockedPopupContainer(); @@ -96,31 +98,21 @@ class BlockedPopupContainer : public ConstrainedWindow, static BlockedPopupContainer* Create( TabContents* owner, Profile* profile, const gfx::Point& initial_anchor); - // Toggles the preference to display this notification. - void ToggleBlockedPopupNotification(); - - // Gets the current state of the show blocked popup notification preference. - bool GetShowBlockedPopupNotification(); - - // Adds a Tabbed contents to this container. |bounds| are the window bounds - // requested by the popup window. - void AddTabContents(TabContents* blocked_contents, + // Adds a popup to this container. |bounds| are the window bounds requested by + // the popup window. + void AddTabContents(TabContents* tab_contents, const gfx::Rect& bounds, const std::string& host); - // Called when a popup from whitelisted host |host| is opened, so we can show - // the "stop whitelisting" UI. - void OnPopupOpenedFromWhitelistedHost(const std::string& host); - - // Creates a window from blocked popup |index|. - void LaunchPopupIndex(int index); + // Shows the blocked popup at index |index|. + void LaunchPopupAtIndex(size_t index); // Returns the number of blocked popups - int GetBlockedPopupCount() const; + size_t GetBlockedPopupCount() const; // Returns the URL and title for popup |index|, used to construct a string for // display. - void GetURLAndTitleForPopup(int index, + void GetURLAndTitleForPopup(size_t index, std::wstring* url, std::wstring* title) const; @@ -129,11 +121,11 @@ class BlockedPopupContainer : public ConstrainedWindow, // Returns true if host |index| is whitelisted. Returns false if |index| is // invalid. - bool IsHostWhitelisted(int index) const; + 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. - void ToggleWhitelistingForHost(int index); + void ToggleWhitelistingForHost(size_t index); // Deletes all popups and hides the interface parts. void CloseAll(); @@ -156,8 +148,10 @@ class BlockedPopupContainer : public ConstrainedWindow, virtual void DidBecomeSelected() { } // Debugging accessors only called from the unit tests. - virtual std::wstring GetWindowTitle() const; - virtual const gfx::Rect& GetCurrentBounds() const; + virtual std::wstring GetWindowTitle() const { + return container_view_->label(); + } + virtual const gfx::Rect& GetCurrentBounds() const { return bounds_; } // Overridden from TabContentsDelegate: @@ -210,22 +204,6 @@ class BlockedPopupContainer : public ConstrainedWindow, RenderViewHost* render_view_host, const std::string& extension_id); - // Overridden from Animation: - - // Changes the visibility percentage of the BlockedPopupContainer. This is - // called while animating in or out. - virtual void AnimateToState(double state); - - protected: - // Overridden from views::ContainerWin: - - // Alerts our |owner_| that we are closing ourselves. Cleans up any remaining - // blocked popups. - virtual void OnFinalMessage(HWND window); - - // Makes the top corners of the window rounded during resizing events. - virtual void OnSize(UINT param, const CSize& size); - private: struct BlockedPopup { BlockedPopup(TabContents* tab_contents, @@ -240,12 +218,34 @@ class BlockedPopupContainer : public ConstrainedWindow, }; typedef std::vector<BlockedPopup> BlockedPopups; + // TabContents is the popup contents. string is opener hostname. + typedef std::map<TabContents*, std::string> UnblockedPopups; + // string is hostname. bool is whitelisted status. typedef std::map<std::string, bool> PopupHosts; // Creates a container for a certain TabContents. BlockedPopupContainer(TabContents* owner, Profile* profile); + // Overridden from Animation: + // Changes the visibility percentage of the BlockedPopupContainer. This is + // called while animating in or out. + virtual void AnimateToState(double state); + + // Overridden from notificationObserver: + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + + // Overridden from views::WidgetWin: + + // Alerts our |owner_| that we are closing ourselves. Cleans up any remaining + // blocked popups. + virtual void OnFinalMessage(HWND window); + + // Makes the top corners of the window rounded during resizing events. + virtual void OnSize(UINT param, const CSize& size); + // Initializes our Views and positions us to the lower right corner of the // browser window. void Init(const gfx::Point& initial_anchor); @@ -253,9 +253,6 @@ class BlockedPopupContainer : public ConstrainedWindow, // Hides the UI portion of the container. void HideSelf(); - // Shows the UI portion of the container. - void ShowSelf(); - // Sets our position, based on our |anchor_point_| and on our // |visibility_percentage_|. This method is called whenever either of those // change. @@ -267,29 +264,36 @@ class BlockedPopupContainer : public ConstrainedWindow, // Helper function to convert a host index (which the view uses) into an // iterator into |popup_hosts_|. Returns popup_hosts_.end() if |index| is // invalid. - PopupHosts::const_iterator ConvertHostIndexToIterator(int index) const; + PopupHosts::const_iterator ConvertHostIndexToIterator(size_t index) const; - // If the popup at |i| is the last one associated with its host, removes the - // host from the host list. - void EraseHostIfNeeded(BlockedPopups::iterator i); + // Removes the popup at |i| from the blocked popup list. If this popup's host + // is not otherwised referenced on either popup list, removes the host from + // the host list. Updates the view's label to match the new state. + void EraseDataForPopupAndUpdateUI(BlockedPopups::iterator i); + + // Same as above, but works on the unblocked popup list, and returns the + // iterator that results from calling erase(). + UnblockedPopups::iterator EraseDataForPopupAndUpdateUI( + UnblockedPopups::iterator i); // The TabContents that owns and constrains this BlockedPopupContainer. TabContents* owner_; + // Registrar to handle notifications we care about. + NotificationRegistrar registrar_; + // Information about all blocked popups. BlockedPopups blocked_popups_; + // Information about all unblocked popups. + UnblockedPopups unblocked_popups_; + // Information about all popup hosts. PopupHosts popup_hosts_; // Our associated view object. BlockedPopupContainerView* container_view_; - // Link to the block popups preference. Used to both determine whether we - // should show ourself to the user and to toggle whether we should show this - // notification to the user. - BooleanPrefMember block_popup_pref_; - // Once the container is hidden, this is set to prevent it from reappearing. bool has_been_dismissed_; diff --git a/chrome/browser/views/options/advanced_contents_view.cc b/chrome/browser/views/options/advanced_contents_view.cc index fb2862b..aed2aea 100644 --- a/chrome/browser/views/options/advanced_contents_view.cc +++ b/chrome/browser/views/options/advanced_contents_view.cc @@ -644,40 +644,24 @@ class WebContentSection : public AdvancedSection, protected: // OptionsPageView overrides: virtual void InitControlLayout(); - virtual void NotifyPrefChanged(const std::wstring* pref_name); private: // Controls for this section: - views::Checkbox* popup_blocked_notification_checkbox_; views::Label* gears_label_; views::NativeButton* gears_settings_button_; - BooleanPrefMember disable_popup_blocked_notification_pref_; - DISALLOW_COPY_AND_ASSIGN(WebContentSection); }; WebContentSection::WebContentSection(Profile* profile) - : popup_blocked_notification_checkbox_(NULL), - gears_label_(NULL), + : gears_label_(NULL), gears_settings_button_(NULL), AdvancedSection(profile, l10n_util::GetString(IDS_OPTIONS_ADVANCED_SECTION_TITLE_CONTENT)) { } void WebContentSection::ButtonPressed(views::Button* sender) { - if (sender == popup_blocked_notification_checkbox_) { - bool notification_disabled = - popup_blocked_notification_checkbox_->checked(); - if (notification_disabled) { - UserMetricsRecordAction(L"Options_BlockAllPopups_Disable", - profile()->GetPrefs()); - } else { - UserMetricsRecordAction(L"Options_BlockAllPopups_Enable", - profile()->GetPrefs()); - } - disable_popup_blocked_notification_pref_.SetValue(!notification_disabled); - } else if (sender == gears_settings_button_) { + if (sender == gears_settings_button_) { UserMetricsRecordAction(L"Options_GearsSettings", NULL); GearsSettingsPressed(GetAncestor(GetWidget()->GetNativeView(), GA_ROOT)); } @@ -686,10 +670,6 @@ void WebContentSection::ButtonPressed(views::Button* sender) { void WebContentSection::InitControlLayout() { AdvancedSection::InitControlLayout(); - popup_blocked_notification_checkbox_ = new views::Checkbox( - l10n_util::GetString(IDS_OPTIONS_SHOWPOPUPBLOCKEDNOTIFICATION)); - popup_blocked_notification_checkbox_->set_listener(this); - if (l10n_util::GetTextDirection() == l10n_util::LEFT_TO_RIGHT) { gears_label_ = new views::Label( l10n_util::GetString(IDS_OPTIONS_GEARSSETTINGS_GROUP_NAME)); @@ -710,25 +690,10 @@ void WebContentSection::InitControlLayout() { contents_->SetLayoutManager(layout); const int col_id = 0; - AddWrappingColumnSet(layout, col_id); - const int two_col_id = 1; - AddTwoColumnSet(layout, two_col_id); + AddTwoColumnSet(layout, col_id); - AddWrappingCheckboxRow(layout, popup_blocked_notification_checkbox_, - col_id, true); AddTwoColumnRow(layout, gears_label_, gears_settings_button_, false, - two_col_id, false); - - // Init member prefs so we can update the controls if prefs change. - disable_popup_blocked_notification_pref_.Init(prefs::kBlockPopups, - profile()->GetPrefs(), this); -} - -void WebContentSection::NotifyPrefChanged(const std::wstring* pref_name) { - if (!pref_name || *pref_name == prefs::kBlockPopups) { - popup_blocked_notification_checkbox_->SetChecked( - !disable_popup_blocked_notification_pref_.GetValue()); - } + col_id, false); } //////////////////////////////////////////////////////////////////////////////// diff --git a/chrome/browser/views/options/advanced_page_view.cc b/chrome/browser/views/options/advanced_page_view.cc index 7b0e1c9..ac0fed1 100644 --- a/chrome/browser/views/options/advanced_page_view.cc +++ b/chrome/browser/views/options/advanced_page_view.cc @@ -105,7 +105,6 @@ void AdvancedPageView::ResetToDefaults() { const wchar_t* kUserPrefs[] = { prefs::kAcceptLanguages, prefs::kAlternateErrorPagesEnabled, - prefs::kBlockPopups, prefs::kCookieBehavior, prefs::kDefaultCharset, prefs::kDnsPrefetchingEnabled, diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 1cc0f1c..7ab6f8f 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -170,10 +170,6 @@ const wchar_t kDefaultSearchProviderName[] = L"default_search_provider.name"; // The id of the default search provider. const wchar_t kDefaultSearchProviderID[] = L"default_search_provider.id"; -// Boolean of whether or not popups should be completely blocked (true), or -// just opened "minimized" (default, false). -const wchar_t kBlockPopups[] = L"browser.block_popups"; - // Boolean which specifies whether we should ask the user if we should download // a file (true) or just download it automatically. const wchar_t kPromptForDownload[] = L"download.prompt_for_download"; diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 2dccc57..a0c1795 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -57,7 +57,6 @@ extern const wchar_t kDefaultSearchProviderSearchURL[]; extern const wchar_t kDefaultSearchProviderSuggestURL[]; extern const wchar_t kDefaultSearchProviderName[]; extern const wchar_t kDefaultSearchProviderID[]; -extern const wchar_t kBlockPopups[]; extern const wchar_t kPromptForDownload[]; extern const wchar_t kAlternateErrorPagesEnabled[]; extern const wchar_t kDnsPrefetchingEnabled[]; |