Commit 92a12e82 authored by Maksim Moskvitin's avatar Maksim Moskvitin Committed by Commit Bot

Remove SubscriptionEntry::Cancel()

This method does nothing what won't be done by dtor and always followed
by destruction.

Bug: 1020117
Change-Id: I1c7a615a0127211a413953fc42da0951aa28c53f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041756Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#739009}
parent e4c4597c
...@@ -167,11 +167,12 @@ struct PerUserTopicSubscriptionManager::SubscriptionEntry { ...@@ -167,11 +167,12 @@ struct PerUserTopicSubscriptionManager::SubscriptionEntry {
SubscriptionFinishedCallback completion_callback, SubscriptionFinishedCallback completion_callback,
PerUserTopicSubscriptionRequest::RequestType type, PerUserTopicSubscriptionRequest::RequestType type,
bool topic_is_public = false); bool topic_is_public = false);
// Destruction of this object causes cancellation of the request.
~SubscriptionEntry(); ~SubscriptionEntry();
void SubscriptionFinished(const Status& code, void SubscriptionFinished(const Status& code,
const std::string& private_topic_name); const std::string& private_topic_name);
void Cancel();
// The object for which this is the status. // The object for which this is the status.
const Topic topic; const Topic topic;
...@@ -208,11 +209,6 @@ void PerUserTopicSubscriptionManager::SubscriptionEntry::SubscriptionFinished( ...@@ -208,11 +209,6 @@ void PerUserTopicSubscriptionManager::SubscriptionEntry::SubscriptionFinished(
completion_callback.Run(topic, code, topic_name, type); completion_callback.Run(topic, code, topic_name, type);
} }
void PerUserTopicSubscriptionManager::SubscriptionEntry::Cancel() {
request_retry_timer_.Stop();
request.reset();
}
PerUserTopicSubscriptionManager::PerUserTopicSubscriptionManager( PerUserTopicSubscriptionManager::PerUserTopicSubscriptionManager(
invalidation::IdentityProvider* identity_provider, invalidation::IdentityProvider* identity_provider,
PrefService* pref_service, PrefService* pref_service,
...@@ -283,11 +279,8 @@ void PerUserTopicSubscriptionManager::UpdateSubscribedTopics( ...@@ -283,11 +279,8 @@ void PerUserTopicSubscriptionManager::UpdateSubscribedTopics(
// If the topic isn't subscribed yet, schedule the subscription. // If the topic isn't subscribed yet, schedule the subscription.
if (topic_to_private_topic_.find(topic.first) == if (topic_to_private_topic_.find(topic.first) ==
topic_to_private_topic_.end()) { topic_to_private_topic_.end()) {
// If there's already a pending request for this topic, cancel it first. // If there was already a pending request for this topic, it'll get
auto it = pending_subscriptions_.find(topic.first); // destroyed and replaced by the new one.
if (it != pending_subscriptions_.end())
it->second->Cancel();
pending_subscriptions_[topic.first] = std::make_unique<SubscriptionEntry>( pending_subscriptions_[topic.first] = std::make_unique<SubscriptionEntry>(
topic.first, topic.first,
base::BindRepeating( base::BindRepeating(
...@@ -304,8 +297,8 @@ void PerUserTopicSubscriptionManager::UpdateSubscribedTopics( ...@@ -304,8 +297,8 @@ void PerUserTopicSubscriptionManager::UpdateSubscribedTopics(
it != topic_to_private_topic_.end();) { it != topic_to_private_topic_.end();) {
Topic topic = it->first; Topic topic = it->first;
if (topics.find(topic) == topics.end()) { if (topics.find(topic) == topics.end()) {
// TODO(crbug.com/1020117): If there's already a pending request for this // If there was already a pending request for this topic, it'll get
// topic, we should probably cancel it first? // destroyed and replaced by the new one.
pending_subscriptions_[topic] = std::make_unique<SubscriptionEntry>( pending_subscriptions_[topic] = std::make_unique<SubscriptionEntry>(
topic, topic,
base::BindRepeating( base::BindRepeating(
...@@ -352,10 +345,11 @@ void PerUserTopicSubscriptionManager::StartPendingSubscriptionRequest( ...@@ -352,10 +345,11 @@ void PerUserTopicSubscriptionManager::StartPendingSubscriptionRequest(
<< " which is not in the registration map"; << " which is not in the registration map";
return; return;
} }
// If retry was scheduled, it will end up in this function and redundant
// request cancellation.
it->second->request_retry_timer_.Stop();
PerUserTopicSubscriptionRequest::Builder builder; PerUserTopicSubscriptionRequest::Builder builder;
// Resetting request in case it's running.
// TODO(crbug.com/1020117): Should probably call it->second->Cancel() instead.
it->second->request.reset();
it->second->request = builder.SetInstanceIdToken(instance_id_token_) it->second->request = builder.SetInstanceIdToken(instance_id_token_)
.SetScope(kInvalidationRegistrationScope) .SetScope(kInvalidationRegistrationScope)
.SetPublicTopicName(topic) .SetPublicTopicName(topic)
...@@ -573,10 +567,6 @@ PerUserTopicSubscriptionManager::DropAllSavedSubscriptionsOnTokenChangeImpl() { ...@@ -573,10 +567,6 @@ PerUserTopicSubscriptionManager::DropAllSavedSubscriptionsOnTokenChangeImpl() {
*update = base::Value(base::Value::Type::DICTIONARY); *update = base::Value(base::Value::Type::DICTIONARY);
topic_to_private_topic_.clear(); topic_to_private_topic_.clear();
private_topic_to_topic_.clear(); private_topic_to_topic_.clear();
// Also cancel any pending subscription requests.
for (const auto& pending_subscription : pending_subscriptions_) {
pending_subscription.second->Cancel();
}
pending_subscriptions_.clear(); pending_subscriptions_.clear();
return instance_id_token_.empty() return instance_id_token_.empty()
? TokenStateOnSubscriptionRequest::kTokenCleared ? TokenStateOnSubscriptionRequest::kTokenCleared
......
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