Commit ca12d264 authored by Carlos IL's avatar Carlos IL Committed by Commit Bot

Use ScopedObserver in MergeSessionNavigationThrottle

Bug: 875284
Change-Id: Id802fe60adaa4f49af2105fffedca7a024c0c82d
Reviewed-on: https://chromium-review.googlesource.com/1211107
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589304}
parent f40d7aaa
......@@ -40,7 +40,7 @@ MergeSessionNavigationThrottle::Create(content::NavigationHandle* handle) {
MergeSessionNavigationThrottle::MergeSessionNavigationThrottle(
content::NavigationHandle* handle)
: NavigationThrottle(handle) {
: NavigationThrottle(handle), login_manager_observer_(this) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
}
......@@ -87,7 +87,7 @@ bool MergeSessionNavigationThrottle::BeforeDefer() {
chromeos::OAuth2LoginManager* manager =
GetOAuth2LoginManager(navigation_handle()->GetWebContents());
if (manager && manager->ShouldBlockTabLoading()) {
manager->AddObserver(this);
login_manager_observer_.Add(manager);
proceed_timer_.Start(FROM_HERE, kTotalWaitTime, this,
&MergeSessionNavigationThrottle::Proceed);
return true;
......@@ -100,7 +100,7 @@ void MergeSessionNavigationThrottle::Proceed() {
chromeos::OAuth2LoginManager* manager =
GetOAuth2LoginManager(navigation_handle()->GetWebContents());
if (manager) {
manager->RemoveObserver(this);
login_manager_observer_.Remove(manager);
}
Resume();
}
......@@ -9,6 +9,7 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h"
#include "base/timer/timer.h"
#include "chrome/browser/chromeos/login/signin/oauth2_login_manager.h"
#include "content/public/browser/navigation_throttle.h"
......@@ -52,6 +53,10 @@ class MergeSessionNavigationThrottle
// navigation.
void Proceed();
ScopedObserver<chromeos::OAuth2LoginManager,
chromeos::OAuth2LoginManager::Observer>
login_manager_observer_;
base::OneShotTimer proceed_timer_;
DISALLOW_COPY_AND_ASSIGN(MergeSessionNavigationThrottle);
......
......@@ -29,6 +29,15 @@ class MergeSessionNavigationThrottleTest
public:
MergeSessionNavigationThrottleTest() = default;
bool ManagerHasObservers() {
OAuth2LoginManager* manager = GetOAuth2LoginManager();
if (!manager) {
ADD_FAILURE();
return false;
}
return manager->observer_list_.might_have_observers();
}
protected:
void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp();
......@@ -89,6 +98,7 @@ TEST_F(MergeSessionNavigationThrottleTest, NotThrottled) {
GURL(kURL), web_contents());
navigation->Start();
EXPECT_FALSE(navigation->IsDeferred());
EXPECT_FALSE(ManagerHasObservers());
}
// Tests navigations are deferred when merge session is in progress, and
......@@ -104,6 +114,7 @@ TEST_F(MergeSessionNavigationThrottleTest, Throttled) {
SetMergeSessionState(OAuth2LoginManager::SESSION_RESTORE_DONE);
navigation->Wait();
EXPECT_FALSE(navigation->IsDeferred());
EXPECT_FALSE(ManagerHasObservers());
}
// Tests navigations are not deferred if merge session started over 60
......@@ -116,6 +127,7 @@ TEST_F(MergeSessionNavigationThrottleTest, Timeout) {
GURL(kURL), web_contents());
navigation->Start();
EXPECT_FALSE(navigation->IsDeferred());
EXPECT_FALSE(ManagerHasObservers());
}
} // namespace chromeos
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