Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Revert of Add GCMChannelStatusSyncer to schedule requests and enable/…
Browse files Browse the repository at this point in the history
…disable GCM (patchset #8 id:150001 of https://codereview.chromium.org/561943002/)

Reason for revert:
this CL might be breaking browser_tests and interactive_ui_tests on ChromeOS because it's starting a request on startup.  here's an example:

[15345:15369:0918/132152:ERROR:leak_tracker.h(97)] Leaked 0x11cd70da7cc0 which was allocated by:
[15345:15369:0918/132152:ERROR:leak_tracker.h(98)] #0 0x7f44b584025e base::debug::StackTrace::StackTrace()
#1 0x000000dfc481 base::debug::LeakTracker<>::LeakTracker()
#2 0x000000df3e8d SystemURLRequestContextGetter::SystemURLRequestContextGetter()
#3 0x000000df57f1 IOThread::InitSystemRequestContext()
#4 0x000000df570b IOThread::system_url_request_context_getter()
#5 0x000001443d9e BrowserProcessImpl::system_request_context()
#6 0x000001028a8e SafeBrowsingService::Initialize()
#7 0x0000014462c9 BrowserProcessImpl::CreateSafeBrowsingService()
#8 0x000001446196 BrowserProcessImpl::safe_browsing_service()
#9 0x0000014b9a06 ChromeResourceDispatcherHostDelegate::ChromeResourceDispatcherHostDelegate()
#10 0x0000014466dc BrowserProcessImpl::ResourceDispatcherHostCreated()
#11 0x0000008a88e5 chrome::ChromeContentBrowserClient::ResourceDispatcherHostCreated()
#12 0x7f44be489a2f content::ResourceDispatcherHostImpl::ResourceDispatcherHostImpl()
#13 0x7f44be060f44 content::BrowserMainLoop::BrowserThreadsStarted()
#14 0x7f44be068302 base::internal::RunnableAdapter<>::Run()
#15 0x7f44be06826c base::internal::InvokeHelper<>::MakeItSo()
#16 0x7f44be06821a base::internal::Invoker<>::Run()
#17 0x7f44be54e46e base::Callback<>::Run()
#18 0x7f44be8d223b content::StartupTaskRunner::RunAllTasksNow()
#19 0x7f44be05ff97 content::BrowserMainLoop::CreateStartupTasks()
#20 0x7f44be06b824 content::BrowserMainRunnerImpl::Initialize()
#21 0x7f44be05c7df content::BrowserMain()
#22 0x7f44bdee5f6f content::RunNamedProcessTypeMain()
#23 0x7f44bdee9268 content::ContentMainRunnerImpl::Run()
#24 0x7f44bdee5505 content::ContentMain()
#25 0x0000041aa8cb content::BrowserTestBase::SetUp()
#26 0x00000173e117 InProcessBrowserTest::SetUp()
#27 0x000000671180 ExtensionBrowserTest::SetUp()
#28 0x0000006711b2 ExtensionBrowserTest::SetUp()
#29 0x0000017fb993 testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#30 0x0000017e8bfe testing::internal::HandleExceptionsInMethodIfSupported<>()
#31 0x0000017dd1e3 testing::Test::Run()
#32 0x0000017dd92b testing::TestInfo::Run()
#33 0x0000017ddf1a testing::TestCase::Run()
#34 0x0000017e3413 testing::internal::UnitTestImpl::RunAllTests()
#35 0x0000017f47d3 testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#36 0x0000017ea96e testing::internal::HandleExceptionsInMethodIfSupported<>()
#37 0x0000017e30b1 testing::UnitTest::Run()
#38 0x000004187e41 RUN_ALL_TESTS()
#39 0x000004186e67 base::TestSuite::Run()
#40 0x0000007fcd62 InteractiveUITestSuiteRunner::RunTestSuite()
#41 0x00000173c7b8 (anonymous namespace)::ChromeTestLauncherDelegate::RunTestSuite()
#42 0x0000041bc2fb content::LaunchTests()
#43 0x00000173c6e9 LaunchChromeTests()
#44 0x0000007fccbf main
#45 0x7f44ad8ed76d __libc_start_main
#46 0x000000601389 <unknown>

[15345:15369:0918/132152:FATAL:leak_tracker.h(102)] Check failed: 0u == count (0 vs. 1)
#0 0x7f44b584025e base::debug::StackTrace::StackTrace()
#1 0x7f44b58d7a62 logging::LogMessage::~LogMessage()
#2 0x000000dfdd01 base::debug::LeakTracker<>::CheckForLeaks()
#3 0x000000df8e13 IOThread::CleanUp()
#4 0x7f44be0810b6 content::BrowserThreadImpl::CleanUp()
#5 0x7f44be07e852 content::BrowserProcessSubThread::CleanUp()
#6 0x7f44b59d0601 base::Thread::ThreadMain()
#7 0x7f44b59bba3c base::(anonymous namespace)::ThreadFunc()
#8 0x7f44b1aa9e9a start_thread
#9 0x7f44ad9c03fd clone

Original issue's description:
> Add GCMChannelStatusSyncer to schedule requests and enable/disable GCM
>
> BUG=384041
> TEST=new tests added
>
> Committed: https://crrev.com/3c23f4a188e171998f3042ad62f4aa5717e66d63
> Cr-Commit-Position: refs/heads/master@{#295524}

[email protected],[email protected],[email protected],[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=384041

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

Cr-Commit-Position: refs/heads/master@{#295549}
  • Loading branch information
danbeam authored and Commit bot committed Sep 18, 2014
1 parent aeae967 commit c5fd179
Show file tree
Hide file tree
Showing 16 changed files with 21 additions and 600 deletions.
1 change: 0 additions & 1 deletion chrome/browser/browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1135,7 +1135,6 @@ void BrowserProcessImpl::CreateGCMDriver() {
CHECK(PathService::Get(chrome::DIR_GLOBAL_GCM_STORE, &store_path));
gcm_driver_ = gcm::CreateGCMDriverDesktop(
make_scoped_ptr(new gcm::GCMClientFactory),
local_state(),
store_path,
system_request_context());
// Sign-in is not required for device-level GCM usage. So we just call
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/prefs/browser_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
#include "components/autofill/core/browser/autofill_manager.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "components/dom_distiller/core/distilled_page_prefs.h"
#include "components/gcm_driver/gcm_channel_status_syncer.h"
#include "components/google/core/browser/google_pref_names.h"
#include "components/google/core/browser/google_url_tracker.h"
#include "components/network_time/network_time_tracker.h"
Expand Down Expand Up @@ -294,8 +293,6 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
AutomaticProfileResetterFactory::RegisterPrefs(registry);
BackgroundModeManager::RegisterPrefs(registry);
RegisterBrowserPrefs(registry);
// The native GCM is used on Android instead.
gcm::GCMChannelStatusSyncer::RegisterPrefs(registry);
#if !defined(OS_CHROMEOS)
RegisterDefaultBrowserPromptPrefs(registry);
#endif // !defined(OS_CHROMEOS)
Expand Down Expand Up @@ -446,7 +443,6 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
extensions::ExtensionSettingsHandler::RegisterProfilePrefs(registry);
extensions::TabsCaptureVisibleTabFunction::RegisterProfilePrefs(registry);
first_run::RegisterProfilePrefs(registry);
gcm::GCMChannelStatusSyncer::RegisterProfilePrefs(registry);
NewTabUI::RegisterProfilePrefs(registry);
PepperFlashSettingsManager::RegisterProfilePrefs(registry);
PinnedTabCodec::RegisterProfilePrefs(registry);
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/services/gcm/gcm_desktop_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ GCMClient::ChromeBuildInfo GetChromeBuildInfo() {

scoped_ptr<GCMDriver> CreateGCMDriverDesktop(
scoped_ptr<GCMClientFactory> gcm_client_factory,
PrefService* prefs,
const base::FilePath& store_path,
const scoped_refptr<net::URLRequestContextGetter>& request_context) {
scoped_refptr<base::SequencedWorkerPool> worker_pool(
Expand All @@ -85,7 +84,6 @@ scoped_ptr<GCMDriver> CreateGCMDriverDesktop(
return scoped_ptr<GCMDriver>(new GCMDriverDesktop(
gcm_client_factory.Pass(),
GetChromeBuildInfo(),
prefs,
store_path,
request_context,
content::BrowserThread::GetMessageLoopProxyForThread(
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/services/gcm/gcm_desktop_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"

class PrefService;
namespace base {
class FilePath;
}
Expand All @@ -24,7 +23,6 @@ class GCMClientFactory;

scoped_ptr<GCMDriver> CreateGCMDriverDesktop(
scoped_ptr<GCMClientFactory> gcm_client_factory,
PrefService* prefs,
const base::FilePath& store_path,
const scoped_refptr<net::URLRequestContextGetter>& request_context);

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/services/gcm/gcm_profile_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ GCMProfileService::GCMProfileService(

driver_ = CreateGCMDriverDesktop(
gcm_client_factory.Pass(),
profile_->GetPrefs(),
profile_->GetPath().Append(chrome::kGCMStoreDirname),
profile_->GetRequestContext());

Expand Down
4 changes: 0 additions & 4 deletions components/gcm_driver.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
'gcm_driver/gcm_backoff_policy.h',
'gcm_driver/gcm_channel_status_request.cc',
'gcm_driver/gcm_channel_status_request.h',
'gcm_driver/gcm_channel_status_syncer.cc',
'gcm_driver/gcm_channel_status_syncer.h',
'gcm_driver/gcm_client.cc',
'gcm_driver/gcm_client.h',
'gcm_driver/gcm_client_factory.cc',
Expand Down Expand Up @@ -75,8 +73,6 @@
'gcm_driver/gcm_account_mapper.h',
'gcm_driver/gcm_channel_status_request.cc',
'gcm_driver/gcm_channel_status_request.h',
'gcm_driver/gcm_channel_status_syncer.cc',
'gcm_driver/gcm_channel_status_syncer.h',
'gcm_driver/gcm_client_factory.cc',
'gcm_driver/gcm_client_factory.h',
'gcm_driver/gcm_client_impl.cc',
Expand Down
4 changes: 0 additions & 4 deletions components/gcm_driver/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ static_library("gcm_driver") {
"gcm_backoff_policy.h",
"gcm_channel_status_request.cc",
"gcm_channel_status_request.h",
"gcm_channel_status_syncer.cc",
"gcm_channel_status_syncer.h",
"gcm_client.cc",
"gcm_client.h",
"gcm_client_factory.cc",
Expand Down Expand Up @@ -53,8 +51,6 @@ static_library("gcm_driver") {
sources -= [
"gcm_channel_status_request.cc",
"gcm_channel_status_request.h",
"gcm_channel_status_syncer.cc",
"gcm_channel_status_syncer.h",
"gcm_client_factory.cc",
"gcm_client_factory.h",
"gcm_client_impl.cc",
Expand Down
1 change: 0 additions & 1 deletion components/gcm_driver/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
include_rules = [
"+components/os_crypt",
"+components/pref_registry",
# TODO(johnme): Fix this layering violation.
"!content/public/android/java",
"+google_apis/gaia",
Expand Down
4 changes: 2 additions & 2 deletions components/gcm_driver/gcm_channel_status_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ GCMChannelStatusRequest::~GCMChannelStatusRequest() {
}

// static
int GCMChannelStatusRequest::default_poll_interval_seconds() {
int GCMChannelStatusRequest::default_poll_interval_seconds_for_testing() {
return kDefaultPollIntervalSeconds;
}

// static
int GCMChannelStatusRequest::min_poll_interval_seconds() {
int GCMChannelStatusRequest::min_poll_interval_seconds_for_testing() {
return kMinPollIntervalSeconds;
}

Expand Down
5 changes: 3 additions & 2 deletions components/gcm_driver/gcm_channel_status_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ class GCMChannelStatusRequest : public net::URLFetcherDelegate {

void Start();

static int default_poll_interval_seconds();
static int min_poll_interval_seconds();
// Exposed for testing purpose.
static int default_poll_interval_seconds_for_testing();
static int min_poll_interval_seconds_for_testing();

private:
// Overridden from URLFetcherDelegate:
Expand Down
15 changes: 9 additions & 6 deletions components/gcm_driver/gcm_channel_status_request_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithDisabledStatus) {
EXPECT_TRUE(request_callback_invoked_);
EXPECT_FALSE(enabled_);
EXPECT_EQ(
GCMChannelStatusRequest::default_poll_interval_seconds(),
GCMChannelStatusRequest::default_poll_interval_seconds_for_testing(),
poll_interval_seconds_);
}

Expand All @@ -148,15 +148,16 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithEnabledStatus) {
EXPECT_TRUE(request_callback_invoked_);
EXPECT_TRUE(enabled_);
EXPECT_EQ(
GCMChannelStatusRequest::default_poll_interval_seconds(),
GCMChannelStatusRequest::default_poll_interval_seconds_for_testing(),
poll_interval_seconds_);
}

TEST_F(GCMChannelStatusRequestTest, ResponseWithPollInterval) {
// Setting a poll interval 15 minutes longer than the minimum interval we
// enforce.
int poll_interval_seconds =
GCMChannelStatusRequest::min_poll_interval_seconds() + 15 * 60;
GCMChannelStatusRequest::min_poll_interval_seconds_for_testing() +
15 * 60;
StartRequest();
SetResponseProtoData(NOT_SPECIFIED, poll_interval_seconds);
CompleteFetch();
Expand All @@ -170,20 +171,22 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithShortPollInterval) {
// Setting a poll interval 15 minutes shorter than the minimum interval we
// enforce.
int poll_interval_seconds =
GCMChannelStatusRequest::min_poll_interval_seconds() - 15 * 60;
GCMChannelStatusRequest::min_poll_interval_seconds_for_testing() -
15 * 60;
StartRequest();
SetResponseProtoData(NOT_SPECIFIED, poll_interval_seconds);
CompleteFetch();

EXPECT_TRUE(request_callback_invoked_);
EXPECT_TRUE(enabled_);
EXPECT_EQ(GCMChannelStatusRequest::min_poll_interval_seconds(),
EXPECT_EQ(GCMChannelStatusRequest::min_poll_interval_seconds_for_testing(),
poll_interval_seconds_);
}

TEST_F(GCMChannelStatusRequestTest, ResponseWithDisabledStatusAndPollInterval) {
int poll_interval_seconds =
GCMChannelStatusRequest::min_poll_interval_seconds() + 15 * 60;
GCMChannelStatusRequest::min_poll_interval_seconds_for_testing() +
15 * 60;
StartRequest();
SetResponseProtoData(GCM_DISABLED, poll_interval_seconds);
CompleteFetch();
Expand Down
183 changes: 0 additions & 183 deletions components/gcm_driver/gcm_channel_status_syncer.cc

This file was deleted.

Loading

0 comments on commit c5fd179

Please sign in to comment.