summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorabarth@chromium.org <abarth@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-06-21 18:35:11 +0000
committerabarth@chromium.org <abarth@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-06-21 18:35:11 +0000
commit300dfbd90dfef6ecb06037f1c9599f4780877565 (patch)
treed3fb2c2fc2d59f6857ca5fcd0ac36a037efe01ea
parent9276a31166bad9c5845193b2fb6ae916d8bfdde4 (diff)
downloadchromium_src-300dfbd90dfef6ecb06037f1c9599f4780877565.zip
chromium_src-300dfbd90dfef6ecb06037f1c9599f4780877565.tar.gz
chromium_src-300dfbd90dfef6ecb06037f1c9599f4780877565.tar.bz2
Don't rely on user gestures for deciding when to dismiss infobars.
Previously, we looked at the user gesture state when deciding whether to dismiss infobars. Now that WebKit's user gesture state doesn't lie, we can tell that this behavior is incorrect. Page-specific infobars, like translate, should close whenever the main frame navigates to a new page, regardless of whether that navigation was conducted from a user gesture. BUG=86417 Review URL: http://codereview.chromium.org/7205026 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@89864 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--chrome/browser/omnibox_search_hint.cc2
-rw-r--r--chrome/browser/tab_contents/confirm_infobar_delegate.cc8
-rw-r--r--chrome/browser/tab_contents/confirm_infobar_delegate.h3
-rw-r--r--chrome/browser/tab_contents/infobar_delegate.cc5
-rw-r--r--chrome/browser/tab_contents/infobar_delegate.h4
-rw-r--r--chrome/browser/translate/translate_infobar_delegate.cc2
-rw-r--r--chrome/browser/ui/browser_init.cc2
-rw-r--r--chrome/browser/ui/browser_list.cc2
-rw-r--r--chrome/browser/ui/cocoa/keystone_infobar.mm2
-rw-r--r--content/browser/tab_contents/navigation_controller.cc15
-rw-r--r--content/browser/tab_contents/navigation_controller_unittest.cc1
-rw-r--r--content/browser/tab_contents/navigation_details.cc1
-rw-r--r--content/browser/tab_contents/navigation_details.h19
-rw-r--r--content/browser/tab_contents/tab_contents.cc2
14 files changed, 25 insertions, 43 deletions
diff --git a/chrome/browser/omnibox_search_hint.cc b/chrome/browser/omnibox_search_hint.cc
index 2e04a3b..a5b8918a 100644
--- a/chrome/browser/omnibox_search_hint.cc
+++ b/chrome/browser/omnibox_search_hint.cc
@@ -102,7 +102,7 @@ HintInfoBar::~HintInfoBar() {
bool HintInfoBar::ShouldExpire(
const content::LoadCommittedDetails& details) const {
- return details.is_user_initiated_main_frame_load() && should_expire_;
+ return details.is_navigation_to_different_page() && should_expire_;
}
void HintInfoBar::InfoBarDismissed() {
diff --git a/chrome/browser/tab_contents/confirm_infobar_delegate.cc b/chrome/browser/tab_contents/confirm_infobar_delegate.cc
index 21b3497..1299065 100644
--- a/chrome/browser/tab_contents/confirm_infobar_delegate.cc
+++ b/chrome/browser/tab_contents/confirm_infobar_delegate.cc
@@ -4,6 +4,7 @@
#include "chrome/browser/tab_contents/confirm_infobar_delegate.h"
+#include "content/browser/tab_contents/navigation_details.h"
#include "content/browser/tab_contents/tab_contents.h"
#include "grit/generated_resources.h"
#include "ui/base/l10n/l10n_util.h"
@@ -50,6 +51,13 @@ bool ConfirmInfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const {
(confirm_delegate->GetMessageText() == GetMessageText());
}
+bool ConfirmInfoBarDelegate::ShouldExpire(
+ const content::LoadCommittedDetails& details) const {
+ if (details.did_replace_entry)
+ return false;
+ return InfoBarDelegate::ShouldExpire(details);
+}
+
ConfirmInfoBarDelegate* ConfirmInfoBarDelegate::AsConfirmInfoBarDelegate() {
return this;
}
diff --git a/chrome/browser/tab_contents/confirm_infobar_delegate.h b/chrome/browser/tab_contents/confirm_infobar_delegate.h
index 694ff28..76e2ade 100644
--- a/chrome/browser/tab_contents/confirm_infobar_delegate.h
+++ b/chrome/browser/tab_contents/confirm_infobar_delegate.h
@@ -59,6 +59,9 @@ class ConfirmInfoBarDelegate : public InfoBarDelegate {
explicit ConfirmInfoBarDelegate(TabContents* contents);
virtual ~ConfirmInfoBarDelegate();
+ virtual bool ShouldExpire(
+ const content::LoadCommittedDetails& details) const OVERRIDE;
+
private:
// InfoBarDelegate:
virtual InfoBar* CreateInfoBar(TabContentsWrapper* owner) OVERRIDE;
diff --git a/chrome/browser/tab_contents/infobar_delegate.cc b/chrome/browser/tab_contents/infobar_delegate.cc
index 99e717e..0c6aab6 100644
--- a/chrome/browser/tab_contents/infobar_delegate.cc
+++ b/chrome/browser/tab_contents/infobar_delegate.cc
@@ -21,10 +21,7 @@ bool InfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const {
bool InfoBarDelegate::ShouldExpire(
const content::LoadCommittedDetails& details) const {
- // Only hide InfoBars when the user has done something that makes the main
- // frame load. We don't want various automatic or subframe navigations making
- // it disappear.
- if (!details.is_user_initiated_main_frame_load())
+ if (!details.is_navigation_to_different_page())
return false;
return ShouldExpireInternal(details);
diff --git a/chrome/browser/tab_contents/infobar_delegate.h b/chrome/browser/tab_contents/infobar_delegate.h
index 34901ba..b82d33a 100644
--- a/chrome/browser/tab_contents/infobar_delegate.h
+++ b/chrome/browser/tab_contents/infobar_delegate.h
@@ -57,8 +57,8 @@ class InfoBarDelegate {
virtual bool EqualsDelegate(InfoBarDelegate* delegate) const;
// Returns true if the InfoBar should be closed automatically after the page
- // is navigated. The default behavior is to return true if the user initiated
- // navigation somewhere else or reloaded.
+ // is navigated. The default behavior is to return true if the
+ // navigation is to a new page (not including reloads).
virtual bool ShouldExpire(
const content::LoadCommittedDetails& details) const;
diff --git a/chrome/browser/translate/translate_infobar_delegate.cc b/chrome/browser/translate/translate_infobar_delegate.cc
index ee69b0f..5d1e140 100644
--- a/chrome/browser/translate/translate_infobar_delegate.cc
+++ b/chrome/browser/translate/translate_infobar_delegate.cc
@@ -358,7 +358,7 @@ bool TranslateInfoBarDelegate::ShouldExpire(
const content::LoadCommittedDetails& details) const {
// Note: we allow closing this infobar even if the main frame navigation
// was programmatic and not initiated by the user - crbug.com/70261 .
- if (!details.is_user_initiated_main_frame_load() && !details.is_main_frame)
+ if (!details.is_navigation_to_different_page() && !details.is_main_frame)
return false;
return InfoBarDelegate::ShouldExpireInternal(details);
diff --git a/chrome/browser/ui/browser_init.cc b/chrome/browser/ui/browser_init.cc
index 924ec7a..158a18c 100644
--- a/chrome/browser/ui/browser_init.cc
+++ b/chrome/browser/ui/browser_init.cc
@@ -187,7 +187,7 @@ DefaultBrowserInfoBarDelegate::~DefaultBrowserInfoBarDelegate() {
bool DefaultBrowserInfoBarDelegate::ShouldExpire(
const content::LoadCommittedDetails& details) const {
- return details.is_user_initiated_main_frame_load() && should_expire_;
+ return details.is_navigation_to_different_page() && should_expire_;
}
gfx::Image* DefaultBrowserInfoBarDelegate::GetIcon() const {
diff --git a/chrome/browser/ui/browser_list.cc b/chrome/browser/ui/browser_list.cc
index 5d25a42..764654a 100644
--- a/chrome/browser/ui/browser_list.cc
+++ b/chrome/browser/ui/browser_list.cc
@@ -53,7 +53,7 @@ class BrowserActivityObserver : public NotificationObserver {
DCHECK(type == NotificationType::NAV_ENTRY_COMMITTED);
const content::LoadCommittedDetails& load =
*Details<content::LoadCommittedDetails>(details).ptr();
- if (!load.is_main_frame || load.is_auto || load.is_in_page)
+ if (!load.is_navigation_to_different_page())
return; // Don't log for subframes or other trivial types.
LogRenderProcessHostCount();
diff --git a/chrome/browser/ui/cocoa/keystone_infobar.mm b/chrome/browser/ui/cocoa/keystone_infobar.mm
index 86935f8..c999411 100644
--- a/chrome/browser/ui/cocoa/keystone_infobar.mm
+++ b/chrome/browser/ui/cocoa/keystone_infobar.mm
@@ -84,7 +84,7 @@ KeystonePromotionInfoBarDelegate::~KeystonePromotionInfoBarDelegate() {
bool KeystonePromotionInfoBarDelegate::ShouldExpire(
const content::LoadCommittedDetails& details) const {
- return details.is_user_initiated_main_frame_load() && can_expire_;
+ return details.is_navigation_to_different_page() && can_expire_;
}
gfx::Image* KeystonePromotionInfoBarDelegate::GetIcon() const {
diff --git a/content/browser/tab_contents/navigation_controller.cc b/content/browser/tab_contents/navigation_controller.cc
index c4574b7..2455e3f 100644
--- a/content/browser/tab_contents/navigation_controller.cc
+++ b/content/browser/tab_contents/navigation_controller.cc
@@ -560,21 +560,6 @@ bool NavigationController::RendererDidNavigate(
// The active entry's SiteInstance should match our SiteInstance.
DCHECK(active_entry->site_instance() == tab_contents_->GetSiteInstance());
- // WebKit doesn't set the "auto" transition on meta refreshes properly (bug
- // 1051891) so we manually set it for redirects which we normally treat as
- // "non-user-gestures" where we want to update stuff after navigations.
- //
- // Note that the redirect check also checks for a pending entry to
- // differentiate real redirects from browser initiated navigations to a
- // redirected entry. This happens when you hit back to go to a page that was
- // the destination of a redirect, we don't want to treat it as a redirect
- // even though that's what its transition will be. See bug 1117048.
- //
- // TODO(brettw) write a test for this complicated logic.
- details->is_auto = (PageTransition::IsRedirect(params.transition) &&
- !pending_entry()) ||
- params.gesture == NavigationGestureAuto;
-
// Now prep the rest of the details for the notification and broadcast.
details->entry = active_entry;
details->is_main_frame = PageTransition::IsMainFrame(params.transition);
diff --git a/content/browser/tab_contents/navigation_controller_unittest.cc b/content/browser/tab_contents/navigation_controller_unittest.cc
index 4f82e49..a3a7c7e 100644
--- a/content/browser/tab_contents/navigation_controller_unittest.cc
+++ b/content/browser/tab_contents/navigation_controller_unittest.cc
@@ -1020,7 +1020,6 @@ TEST_F(NavigationControllerTest, NewSubframe) {
EXPECT_TRUE(notifications.Check1AndReset(
NotificationType::NAV_ENTRY_COMMITTED));
EXPECT_EQ(url1, details.previous_url);
- EXPECT_FALSE(details.is_auto);
EXPECT_FALSE(details.is_in_page);
EXPECT_FALSE(details.is_main_frame);
diff --git a/content/browser/tab_contents/navigation_details.cc b/content/browser/tab_contents/navigation_details.cc
index 421e86e..ad44208 100644
--- a/content/browser/tab_contents/navigation_details.cc
+++ b/content/browser/tab_contents/navigation_details.cc
@@ -10,7 +10,6 @@ LoadCommittedDetails::LoadCommittedDetails()
: entry(NULL),
type(NavigationType::UNKNOWN),
previous_entry_index(-1),
- is_auto(false),
did_replace_entry(false),
is_in_page(false),
is_main_frame(true),
diff --git a/content/browser/tab_contents/navigation_details.h b/content/browser/tab_contents/navigation_details.h
index 11448c9..2bb3df3 100644
--- a/content/browser/tab_contents/navigation_details.h
+++ b/content/browser/tab_contents/navigation_details.h
@@ -37,16 +37,8 @@ struct LoadCommittedDetails {
// The previous URL that the user was on. This may be empty if none.
GURL previous_url;
- // True when this load was non-user initated. This corresponds to a
- // a NavigationGestureAuto call from WebKit (see webview_delegate.h).
- // We also count reloads and meta-refreshes as "auto" to account for the
- // fact that WebKit doesn't always set the user gesture properly in these
- // cases (see bug 1051891).
- bool is_auto;
-
// True if the committed entry has replaced the exisiting one.
// A non-user initiated redirect causes such replacement.
- // This is somewhat similiar to is_auto, but not exactly the same.
bool did_replace_entry;
// True if the navigation was in-page. This means that the active entry's
@@ -63,12 +55,11 @@ struct LoadCommittedDetails {
// Use SSLManager::DeserializeSecurityInfo to decode it.
std::string serialized_security_info;
- // Returns whether the user probably felt like they navigated somewhere new.
- // We often need this logic for showing or hiding something, and this
- // returns true only for main frame loads that the user initiated, that go
- // to a new page.
- bool is_user_initiated_main_frame_load() const {
- return !is_auto && !is_in_page && is_main_frame;
+ // Returns whether the main frame navigated to a different page (e.g., not
+ // scrolling to a fragment inside the current page). We often need this logic
+ // for showing or hiding something.
+ bool is_navigation_to_different_page() const {
+ return is_main_frame && !is_in_page;
}
// The HTTP status code for this entry..
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index cc80505..200d80f 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -1061,7 +1061,7 @@ void TabContents::DidNavigateMainFramePostCommit(
opener_web_ui_type_ = WebUI::kNoWebUI;
}
- if (details.is_user_initiated_main_frame_load()) {
+ if (details.is_navigation_to_different_page()) {
// Clear the status bubble. This is a workaround for a bug where WebKit
// doesn't let us know that the cursor left an element during a
// transition (this is also why the mouse cursor remains as a hand after
generated by cgit v1.1 at 2025-03-05 19:04:35 +0000