diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-08 02:03:50 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-08 02:03:50 +0000 |
commit | 0be0993cca7083adc764540ed17ba7611c23aa5a (patch) | |
tree | 2c7337c84594d00957b6d961afa5ee62a12ff6c5 /chrome/browser/custom_handlers | |
parent | d52f52f478d5aa0830cd2b621e464178d2fbaa1f (diff) | |
download | chromium_src-0be0993cca7083adc764540ed17ba7611c23aa5a.zip chromium_src-0be0993cca7083adc764540ed17ba7611c23aa5a.tar.gz chromium_src-0be0993cca7083adc764540ed17ba7611c23aa5a.tar.bz2 |
Change infobar creation to use a public static Create() method on the infobar delegate classes. Make constructors as private as possible.
This has several purposes:
* By preventing direct instantiation, it prevents callers from leaking if they create an infobar and don't add it to an InfoBarService.
* By moving decision-making about when to show infobars into these Create() functions, there's a pattern for where such code should go, and caller code becomes simpler and easier to read.
* The two bullets above together mean that for infobars which should only be displayed in certain circumstances, code can't accidentally bypass the decision logic.
* It enables us to eliminate a common InfoBarService* temp on the caller side since the caller no longer needs to both pass the pointer to the infobar _and_ call AddInfoBar() on the pointer. This was also a somewhat redundant-looking pattern.
* It makes it easier to change the ownership model for infobars in the future by limiting the affected callsites to only the Create() functions.
Note that right now, this still feels pretty redundant since we pass all the same args to Create() functions as constructors most times. In the new ownership model constructors will no longer need to take InfoBarService*s, which will make this better.
Additionally, this makes AddInfoBar()/ReplaceInfoBar() take scoped_ptr<>s to indicate they're receiving ownership. This sort of change is easy to make since we only need change the create functions.
This change also has a functional effect: it eliminates some cases where we tried to only show infobars when no other infobars were already showing (discussed and approved by Glen).
BUG=none
TEST=none
Review URL: https://codereview.chromium.org/11644059
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175467 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/custom_handlers')
-rw-r--r-- | chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.cc | 41 | ||||
-rw-r--r-- | chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h | 15 |
2 files changed, 41 insertions, 15 deletions
diff --git a/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.cc b/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.cc index 0f62ce0..52c2a94 100644 --- a/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.cc +++ b/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.cc @@ -17,13 +17,30 @@ using content::OpenURLParams; using content::Referrer; using content::UserMetricsAction; -RegisterProtocolHandlerInfoBarDelegate::RegisterProtocolHandlerInfoBarDelegate( +// static +void RegisterProtocolHandlerInfoBarDelegate::Create( InfoBarService* infobar_service, ProtocolHandlerRegistry* registry, - const ProtocolHandler& handler) - : ConfirmInfoBarDelegate(infobar_service), - registry_(registry), - handler_(handler) { + const ProtocolHandler& handler) { + content::RecordAction( + content::UserMetricsAction("RegisterProtocolHandler.InfoBar_Shown")); + + scoped_ptr<InfoBarDelegate> infobar( + new RegisterProtocolHandlerInfoBarDelegate(infobar_service, registry, + handler)); + + for (size_t i = 0; i < infobar_service->GetInfoBarCount(); ++i) { + RegisterProtocolHandlerInfoBarDelegate* existing_delegate = + infobar_service->GetInfoBarDelegateAt(i)-> + AsRegisterProtocolHandlerInfoBarDelegate(); + if ((existing_delegate != NULL) && + existing_delegate->handler_.IsEquivalent(handler)) { + infobar_service->ReplaceInfoBar(existing_delegate, infobar.Pass()); + return; + } + } + + infobar_service->AddInfoBar(infobar.Pass()); } InfoBarDelegate::InfoBarAutomationType @@ -47,6 +64,15 @@ string16 RegisterProtocolHandlerInfoBarDelegate::GetMessageText() const { GetProtocolName(handler_)); } +RegisterProtocolHandlerInfoBarDelegate::RegisterProtocolHandlerInfoBarDelegate( + InfoBarService* infobar_service, + ProtocolHandlerRegistry* registry, + const ProtocolHandler& handler) + : ConfirmInfoBarDelegate(infobar_service), + registry_(registry), + handler_(handler) { +} + string16 RegisterProtocolHandlerInfoBarDelegate::GetProtocolName( const ProtocolHandler& handler) const { if (handler.protocol() == "mailto") @@ -101,11 +127,6 @@ bool RegisterProtocolHandlerInfoBarDelegate::LinkClicked( return false; } -bool RegisterProtocolHandlerInfoBarDelegate::IsReplacedBy( - RegisterProtocolHandlerInfoBarDelegate* delegate) { - return handler_.IsEquivalent(delegate->handler_); -} - RegisterProtocolHandlerInfoBarDelegate* RegisterProtocolHandlerInfoBarDelegate:: AsRegisterProtocolHandlerInfoBarDelegate() { diff --git a/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h b/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h index d319d3c..7ea1d4e 100644 --- a/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h +++ b/chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h @@ -16,9 +16,12 @@ class ProtocolHandlerRegistry; // card information gathered from a form submission. class RegisterProtocolHandlerInfoBarDelegate : public ConfirmInfoBarDelegate { public: - RegisterProtocolHandlerInfoBarDelegate(InfoBarService* infobar_service, - ProtocolHandlerRegistry* registry, - const ProtocolHandler& handler); + // Creates a new RPH delegate. Searches |infobar_service| for an existing + // delegate for the same |handler|; replaces it with the new delegate if + // found, otherwise adds the new infobar to |infobar_service|. + static void Create(InfoBarService* infobar_service, + ProtocolHandlerRegistry* registry, + const ProtocolHandler& handler); // ConfirmInfoBarDelegate: virtual Type GetInfoBarType() const OVERRIDE; @@ -35,9 +38,11 @@ class RegisterProtocolHandlerInfoBarDelegate : public ConfirmInfoBarDelegate { virtual InfoBarAutomationType GetInfoBarAutomationType() const OVERRIDE; - bool IsReplacedBy(RegisterProtocolHandlerInfoBarDelegate* delegate); - private: + RegisterProtocolHandlerInfoBarDelegate(InfoBarService* infobar_service, + ProtocolHandlerRegistry* registry, + const ProtocolHandler& handler); + // Returns a user-friendly name for the protocol of this protocol handler. string16 GetProtocolName(const ProtocolHandler& handler) const; ProtocolHandlerRegistry* registry_; |