Skip to content

Commit

Permalink
Pass a WebContents* to infobar delegate Create() functions/constructo…
Browse files Browse the repository at this point in the history
…rs instead

of getting the WebContents from the InfoBarService or using web_contents().  The
former is mainly a simplification, the latter will be illegal once the infobar
refactor lands (and caused the crash in bug 325216).

This will also allow me to subsequently revert r234759 because I can just pass
in the appropriate WebContents*.  (I actually could already do so today, but to
my shame didn't think of doing so.)

BUG=325216
TEST=none
R=sky@chromium.org

Review URL: https://codereview.chromium.org/105503005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239003 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
pkasting@chromium.org committed Dec 5, 2013
1 parent 796b989 commit b8fdbd9
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 75 deletions.
6 changes: 4 additions & 2 deletions chrome/browser/extensions/extension_infobar_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ ExtensionInfoBarDelegate::~ExtensionInfoBarDelegate() {
}

// static
void ExtensionInfoBarDelegate::Create(InfoBarService* infobar_service,
void ExtensionInfoBarDelegate::Create(content::WebContents* web_contents,
Browser* browser,
const extensions::Extension* extension,
const GURL& url,
int height) {
InfoBarService* infobar_service =
InfoBarService::FromWebContents(web_contents);
infobar_service->AddInfoBar(scoped_ptr<InfoBarDelegate>(
new ExtensionInfoBarDelegate(browser, infobar_service, extension, url,
infobar_service->web_contents(), height)));
web_contents, height)));
}

ExtensionInfoBarDelegate::ExtensionInfoBarDelegate(
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/extensions/extension_infobar_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

class Browser;
class GURL;
class InfoBarService;

namespace extensions {
class Extension;
Expand All @@ -35,8 +34,9 @@ class ExtensionInfoBarDelegate : public InfoBarDelegate,

virtual ~ExtensionInfoBarDelegate();

// Creates an extension infobar delegate and adds it to |infobar_service|.
static void Create(InfoBarService* infobar_service,
// Creates an extension infobar delegate and adds it to the infobar service
// for |web_contents|.
static void Create(content::WebContents* web_contents,
Browser* browser,
const extensions::Extension* extension,
const GURL& url,
Expand Down
35 changes: 10 additions & 25 deletions chrome/browser/infobars/infobar_extension_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,60 +12,45 @@
#include "chrome/browser/extensions/extension_infobar_delegate.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/extensions/window_controller.h"
#include "chrome/browser/infobars/confirm_infobar_delegate.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/common/url_constants.h"
#include "content/public/browser/web_contents.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension.h"
#include "grit/generated_resources.h"

using extensions::Extension;

namespace {

const char kHtmlPath[] = "path";
const char kTabId[] = "tabId";
const char kHeight[] = "height";

} // namespace

bool InfobarsShowFunction::RunImpl() {
DictionaryValue* args;
EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &args));

const char kTabId[] = "tabId";
int tab_id;
EXTENSION_FUNCTION_VALIDATE(args->GetInteger(kTabId, &tab_id));

const char kHtmlPath[] = "path";
std::string html_path;
EXTENSION_FUNCTION_VALIDATE(args->GetString(kHtmlPath, &html_path));

const char kHeight[] = "height";
int height = 0;
if (args->HasKey(kHeight))
EXTENSION_FUNCTION_VALIDATE(args->GetInteger(kHeight, &height));

const Extension* extension = GetExtension();
GURL url = extension->GetResourceURL(extension->url(), html_path);

Browser* browser = NULL;
content::WebContents* web_contents = NULL;
if (!extensions::ExtensionTabUtil::GetTabById(tab_id,
GetProfile(),
include_incognito(),
&browser,
NULL,
&web_contents,
NULL)) {
if (!extensions::ExtensionTabUtil::GetTabById(tab_id, GetProfile(),
include_incognito(), &browser,
NULL, &web_contents, NULL)) {
error_ = extensions::ErrorUtils::FormatErrorMessage(
extensions::tabs_constants::kTabNotFoundError,
base::IntToString(tab_id));
return false;
}

ExtensionInfoBarDelegate::Create(
InfoBarService::FromWebContents(web_contents), browser, GetExtension(),
url, height);
const extensions::Extension* extension = GetExtension();
GURL url(extension->GetResourceURL(extension->url(), html_path));
ExtensionInfoBarDelegate::Create(web_contents, browser, GetExtension(), url,
height);

// TODO(finnur): Return the actual DOMWindow object. Bug 26463.
DCHECK(browser->extension_window_controller());
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/translate/translate_infobar_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ TranslateInfoBarDelegate::~TranslateInfoBarDelegate() {
// static
void TranslateInfoBarDelegate::Create(
bool replace_existing_infobar,
InfoBarService* infobar_service,
content::WebContents* web_contents,
Type infobar_type,
const std::string& original_language,
const std::string& target_language,
Expand All @@ -58,13 +58,15 @@ void TranslateInfoBarDelegate::Create(
if ((infobar_type == TranslateInfoBarDelegate::AFTER_TRANSLATE) ||
(infobar_type == TranslateInfoBarDelegate::TRANSLATING)) {
TranslateTabHelper* translate_tab_helper =
TranslateTabHelper::FromWebContents(infobar_service->web_contents());
TranslateTabHelper::FromWebContents(web_contents);
if (!translate_tab_helper ||
translate_tab_helper->language_state().InTranslateNavigation())
return;
}

// Find any existing translate infobar delegate.
InfoBarService* infobar_service =
InfoBarService::FromWebContents(web_contents);
TranslateInfoBarDelegate* old_delegate = NULL;
for (size_t i = 0; i < infobar_service->infobar_count(); ++i) {
old_delegate = infobar_service->infobar_at(i)->AsTranslateInfoBarDelegate();
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/translate/translate_infobar_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
// |original_language| == kUnknownLanguageCode.
//
// If |replace_existing_infobar| is true, the infobar is created and added to
// |infobar_service|, replacing any other translate infobar already present
// there. Otherwise, the infobar will only be added if there is no other
// translate infobar already present.
// the infobar service for |web_contents|, replacing any other translate
// infobar already present there. Otherwise, the infobar will only be added
// if there is no other translate infobar already present.
static void Create(bool replace_existing_infobar,
InfoBarService* infobar_service,
content::WebContents* web_contents,
Type infobar_type,
const std::string& original_language,
const std::string& target_language,
Expand Down
26 changes: 12 additions & 14 deletions chrome/browser/translate/translate_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "base/time/time.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/tab_contents/language_state.h"
#include "chrome/browser/tab_contents/tab_util.h"
Expand Down Expand Up @@ -439,9 +438,9 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
} else {
// Prompts the user if he/she wants the page translated.
TranslateInfoBarDelegate::Create(
false, InfoBarService::FromWebContents(web_contents),
TranslateInfoBarDelegate::BEFORE_TRANSLATE, language_code, target_lang,
TranslateErrors::NONE, profile->GetPrefs(), ShortcutConfig());
false, web_contents, TranslateInfoBarDelegate::BEFORE_TRANSLATE,
language_code, target_lang, TranslateErrors::NONE, profile->GetPrefs(),
ShortcutConfig());
}
}

Expand Down Expand Up @@ -501,9 +500,9 @@ void TranslateManager::TranslatePage(WebContents* web_contents,
Profile* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
TranslateInfoBarDelegate::Create(
true, InfoBarService::FromWebContents(web_contents),
TranslateInfoBarDelegate::TRANSLATING, source_lang, target_lang,
TranslateErrors::NONE, profile->GetPrefs(), ShortcutConfig());
true, web_contents, TranslateInfoBarDelegate::TRANSLATING, source_lang,
target_lang, TranslateErrors::NONE, profile->GetPrefs(),
ShortcutConfig());
}

DCHECK(script_.get() != NULL);
Expand Down Expand Up @@ -626,10 +625,10 @@ void TranslateManager::PageTranslated(WebContents* web_contents,
PrefService* prefs = Profile::FromBrowserContext(
web_contents->GetBrowserContext())->GetPrefs();
TranslateInfoBarDelegate::Create(
true, InfoBarService::FromWebContents(web_contents),
true, web_contents,
(details->error_type == TranslateErrors::NONE) ?
TranslateInfoBarDelegate::AFTER_TRANSLATE :
TranslateInfoBarDelegate::TRANSLATION_ERROR,
TranslateInfoBarDelegate::AFTER_TRANSLATE :
TranslateInfoBarDelegate::TRANSLATION_ERROR,
details->source_language, details->target_language, details->error_type,
prefs, ShortcutConfig());
}
Expand Down Expand Up @@ -697,10 +696,9 @@ void TranslateManager::OnTranslateScriptFetchComplete(
Profile* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
TranslateInfoBarDelegate::Create(
true, InfoBarService::FromWebContents(web_contents),
TranslateInfoBarDelegate::TRANSLATION_ERROR, request.source_lang,
request.target_lang, TranslateErrors::NETWORK, profile->GetPrefs(),
ShortcutConfig());
true, web_contents, TranslateInfoBarDelegate::TRANSLATION_ERROR,
request.source_lang, request.target_lang, TranslateErrors::NETWORK,
profile->GetPrefs(), ShortcutConfig());
}

if (!web_contents->GetBrowserContext()->IsOffTheRecord()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ using base::android::ScopedJavaLocalRef;

AutoLoginInfoBarDelegateAndroid::AutoLoginInfoBarDelegateAndroid(
InfoBarService* owner,
const Params& params)
: AutoLoginInfoBarDelegate(owner, params),
const Params& params,
Profile* profile)
: AutoLoginInfoBarDelegate(owner, params, profile),
params_(params) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@

class AutoLoginInfoBarDelegateAndroid : public AutoLoginInfoBarDelegate {
public:
AutoLoginInfoBarDelegateAndroid(InfoBarService* owner, const Params& params);
AutoLoginInfoBarDelegateAndroid(InfoBarService* owner,
const Params& params,
Profile* profile);
virtual ~AutoLoginInfoBarDelegateAndroid();

// AutoLoginInfoBarDelegate:
Expand Down
25 changes: 17 additions & 8 deletions chrome/browser/ui/auto_login_infobar_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,27 +125,36 @@ void AutoLoginRedirector::RedirectToMergeSession(const std::string& token) {
// AutoLoginInfoBarDelegate ---------------------------------------------------

// static
void AutoLoginInfoBarDelegate::Create(InfoBarService* infobar_service,
bool AutoLoginInfoBarDelegate::Create(content::WebContents* web_contents,
const Params& params) {
infobar_service->AddInfoBar(scoped_ptr<InfoBarDelegate>(
// If |web_contents| is hosted in a WebDialog, there may be no infobar
// service.
InfoBarService* infobar_service =
InfoBarService::FromWebContents(web_contents);
if (!infobar_service)
return false;

Profile* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
return infobar_service->AddInfoBar(scoped_ptr<InfoBarDelegate>(
#if defined(OS_ANDROID)
new AutoLoginInfoBarDelegateAndroid(infobar_service, params)
new AutoLoginInfoBarDelegateAndroid(infobar_service, params, profile)
#else
new AutoLoginInfoBarDelegate(infobar_service, params)
new AutoLoginInfoBarDelegate(infobar_service, params, profile)
#endif
));
)) != NULL;
}

AutoLoginInfoBarDelegate::AutoLoginInfoBarDelegate(
InfoBarService* owner,
const Params& params)
const Params& params,
Profile* profile)
: ConfirmInfoBarDelegate(owner),
params_(params),
button_pressed_(false) {
RecordHistogramAction(SHOWN);
registrar_.Add(this, chrome::NOTIFICATION_GOOGLE_SIGNED_OUT,
content::Source<Profile>(Profile::FromBrowserContext(
web_contents()->GetBrowserContext())));
content::Source<Profile>(profile));
}

AutoLoginInfoBarDelegate::~AutoLoginInfoBarDelegate() {
Expand Down
10 changes: 7 additions & 3 deletions chrome/browser/ui/auto_login_infobar_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "content/public/browser/notification_registrar.h"

class PrefService;
class Profile;

namespace content {
class NavigationController;
Expand All @@ -31,11 +32,14 @@ class AutoLoginInfoBarDelegate : public ConfirmInfoBarDelegate,
std::string username;
};

// Creates an autologin infobar delegate and adds it to |infobar_service|.
static void Create(InfoBarService* infobar_service, const Params& params);
// Creates an autologin infobar delegate and adds it to the infobar service
// for |web_contents|. Returns whether the infobar was successfully created.
static bool Create(content::WebContents* web_contents, const Params& params);

protected:
AutoLoginInfoBarDelegate(InfoBarService* owner, const Params& params);
AutoLoginInfoBarDelegate(InfoBarService* owner,
const Params& params,
Profile* profile);
virtual ~AutoLoginInfoBarDelegate();

private:
Expand Down
13 changes: 2 additions & 11 deletions chrome/browser/ui/auto_login_prompter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "base/logging.h"
#include "base/prefs/pref_service.h"
#include "chrome/browser/google/google_url_tracker.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/profile_oauth2_token_service.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
Expand Down Expand Up @@ -138,14 +137,6 @@ void AutoLoginPrompter::WebContentsDestroyed(WebContents* web_contents) {
}

void AutoLoginPrompter::AddInfoBarToWebContents() {
if (infobar_shown_)
return;

InfoBarService* infobar_service =
InfoBarService::FromWebContents(web_contents());
// |infobar_service| is NULL for WebContents hosted in WebDialog.
if (infobar_service) {
AutoLoginInfoBarDelegate::Create(infobar_service, params_);
infobar_shown_ = true;
}
if (!infobar_shown_)
infobar_shown_ = AutoLoginInfoBarDelegate::Create(web_contents(), params_);
}

0 comments on commit b8fdbd9

Please sign in to comment.