Commit 5d55706e authored by Avi Drissman's avatar Avi Drissman Committed by Commit Bot

Properly remove the "needs attention" indicator for purged tabs.

If the resource coordinator purges a tab, the contents for that
tab will be destroyed after it is removed from the tabstrip.
Watch the tabstrip to make sure to handle that case.

BUG=786178
TEST=as in that case

Change-Id: I3d2c2e9fab29fe639615eed330ca769243c9a8f7
Reviewed-on: https://chromium-review.googlesource.com/777470Reviewed-by: default avatarSidney San Martín <sdy@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517907}
parent 6cc3cfed
...@@ -56,6 +56,7 @@ enum class JavaScriptDialogTabHelper::DismissalCause { ...@@ -56,6 +56,7 @@ enum class JavaScriptDialogTabHelper::DismissalCause {
BROWSER_SWITCHED = 5, BROWSER_SWITCHED = 5,
DIALOG_BUTTON_CLICKED = 6, DIALOG_BUTTON_CLICKED = 6,
TAB_NAVIGATED = 7, TAB_NAVIGATED = 7,
TAB_SWITCHED_OUT = 8,
MAX, MAX,
}; };
...@@ -283,6 +284,21 @@ void JavaScriptDialogTabHelper::OnBrowserSetLastActive(Browser* browser) { ...@@ -283,6 +284,21 @@ void JavaScriptDialogTabHelper::OnBrowserSetLastActive(Browser* browser) {
HandleTabSwitchAway(DismissalCause::BROWSER_SWITCHED); HandleTabSwitchAway(DismissalCause::BROWSER_SWITCHED);
} }
} }
void JavaScriptDialogTabHelper::TabReplacedAt(
TabStripModel* tab_strip_model,
content::WebContents* old_contents,
content::WebContents* new_contents,
int index) {
if (old_contents == WebContentsObserver::web_contents()) {
// At this point, this WebContents is no longer in the tabstrip. The usual
// teardown will not be able to turn off the attention indicator, so that
// must be done here.
SetTabNeedsAttentionImpl(false, tab_strip_model, index);
CloseDialog(DismissalCause::TAB_SWITCHED_OUT, false, base::string16());
}
}
#endif #endif
void JavaScriptDialogTabHelper::LogDialogDismissalCause( void JavaScriptDialogTabHelper::LogDialogDismissalCause(
...@@ -347,8 +363,10 @@ void JavaScriptDialogTabHelper::CloseDialog(DismissalCause cause, ...@@ -347,8 +363,10 @@ void JavaScriptDialogTabHelper::CloseDialog(DismissalCause cause,
std::move(dialog_callback_).Run(success, user_input); std::move(dialog_callback_).Run(success, user_input);
// If there's a pending dialog, then the tab is still in the "needs attention" // If there's a pending dialog, then the tab is still in the "needs attention"
// state; clear it out. // state; clear it out. However, if the tab was switched out, the turning off
if (pending_dialog_) // of the "needs attention" state was done in TabReplacedAt() because
// SetTabNeedsAttention won't work, so don't call it.
if (pending_dialog_ && cause != DismissalCause::TAB_SWITCHED_OUT)
SetTabNeedsAttention(false); SetTabNeedsAttention(false);
dialog_.reset(); dialog_.reset();
...@@ -364,9 +382,23 @@ void JavaScriptDialogTabHelper::SetTabNeedsAttention(bool attention) { ...@@ -364,9 +382,23 @@ void JavaScriptDialogTabHelper::SetTabNeedsAttention(bool attention) {
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
content::WebContents* web_contents = WebContentsObserver::web_contents(); content::WebContents* web_contents = WebContentsObserver::web_contents();
Browser* browser = chrome::FindBrowserWithWebContents(web_contents); Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
DCHECK(browser); TabStripModel* tab_strip_model = browser->tab_strip_model();
browser->tab_strip_model()->SetTabNeedsAttentionAt(
browser->tab_strip_model()->GetIndexOfWebContents(web_contents), SetTabNeedsAttentionImpl(
attention); attention, tab_strip_model,
tab_strip_model->GetIndexOfWebContents(web_contents));
#endif #endif
} }
#if !defined(OS_ANDROID)
void JavaScriptDialogTabHelper::SetTabNeedsAttentionImpl(
bool attention,
TabStripModel* tab_strip_model,
int index) {
tab_strip_model->SetTabNeedsAttentionAt(index, attention);
if (attention)
tab_strip_model->AddObserver(this);
else
tab_strip_model->RemoveObserver(this);
}
#endif
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
#include "chrome/browser/ui/browser_list_observer.h" #include "chrome/browser/ui/browser_list_observer.h"
#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
#endif #endif
// A class, attached to WebContentses in browser windows, that is the // A class, attached to WebContentses in browser windows, that is the
...@@ -43,6 +44,7 @@ class JavaScriptDialogTabHelper ...@@ -43,6 +44,7 @@ class JavaScriptDialogTabHelper
public content::WebContentsObserver, public content::WebContentsObserver,
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
public chrome::BrowserListObserver, public chrome::BrowserListObserver,
public TabStripModelObserver,
#endif #endif
public content::WebContentsUserData<JavaScriptDialogTabHelper> { public content::WebContentsUserData<JavaScriptDialogTabHelper> {
public: public:
...@@ -81,6 +83,12 @@ class JavaScriptDialogTabHelper ...@@ -81,6 +83,12 @@ class JavaScriptDialogTabHelper
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
// BrowserListObserver: // BrowserListObserver:
void OnBrowserSetLastActive(Browser* browser) override; void OnBrowserSetLastActive(Browser* browser) override;
// TabStripModelObserver:
void TabReplacedAt(TabStripModel* tab_strip_model,
content::WebContents* old_contents,
content::WebContents* new_contents,
int index) override;
#endif #endif
private: private:
...@@ -99,9 +107,17 @@ class JavaScriptDialogTabHelper ...@@ -99,9 +107,17 @@ class JavaScriptDialogTabHelper
bool success, bool success,
const base::string16& user_input); const base::string16& user_input);
// Marks the tab as needing attention. // Marks the tab as needing attention. The WebContents must be in a browser
// window.
void SetTabNeedsAttention(bool attention); void SetTabNeedsAttention(bool attention);
#if !defined(OS_ANDROID)
// Marks the tab as needing attention.
void SetTabNeedsAttentionImpl(bool attention,
TabStripModel* tab_strip_model,
int index);
#endif
// There can be at most one dialog (pending or not) being shown at any given // There can be at most one dialog (pending or not) being shown at any given
// time on a tab. Depending on the type of the dialog, the variables // time on a tab. Depending on the type of the dialog, the variables
// |dialog_|, |pending_dialog_|, and |dialog_callback_| can be present in // |dialog_|, |pending_dialog_|, and |dialog_callback_| can be present in
......
...@@ -22574,6 +22574,10 @@ Called by update_gpu_driver_bug_workaround_entries.py.--> ...@@ -22574,6 +22574,10 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
<int value="7" label="Tab navigated"> <int value="7" label="Tab navigated">
The user navigated the tab (e.g. back/forward, reload) The user navigated the tab (e.g. back/forward, reload)
</int> </int>
<int value="8" label="Tab switched out">
The page displaying the dialog was switched out of its tab (e.g. tab
discarding)
</int>
</enum> </enum>
<enum name="JumplisticonsDeleteCategory"> <enum name="JumplisticonsDeleteCategory">
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment