Commit fe877ab0 authored by David Roger's avatar David Roger Committed by Commit Bot

[signin] GaiaCookieManagerService no longer ignores cookie changes

The GCMS was ignoring some cookie change notifications, specifically
the ones receieved while the GCMS is busy.
This was orinally done to fix a bug in the AccountReconcilor (see
https://crbug.com/516070). However this was arguably the wrong place to
fix it, and also it made the GCMS unreliable.
Later, the AccountReconcilor was fixed (https://codereview.chromium.org/1267843003),
and thus this is no longer necessary.

This CL restores the previous behavior, and is essentially a revert
of https://codereview.chromium.org/1264143003/

To completely avoid reconcilor loops, we will do the following:
- rate-limit the reconcilor (bug 1008435)
- add a new histogram that give the detailed status of a reconcilor run.
  The buckets could be something like:
   * no-op
   * failure (ideally including the failure reason)
   * success (logout)
   * success (multilogin)
   * success (MergeSession)
   * success (MergeSession + logout)

Bug: 923873
Change-Id: Ib82cb511ea328793f80293450d140a74a1de6415
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831863Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710290}
parent 2d0a4773
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <stddef.h> #include <stddef.h>
#include <algorithm>
#include <queue> #include <queue>
#include <set> #include <set>
...@@ -674,11 +675,6 @@ void GaiaCookieManagerService::OnCookieChange( ...@@ -674,11 +675,6 @@ void GaiaCookieManagerService::OnCookieChange(
} }
} }
// Ignore changes to the cookie while requests are pending. These changes
// are caused by the service itself as it adds accounts. A side effects is
// that any changes to the gaia cookie outside of this class, while requests
// are pending, will be lost. However, trying to process these changes could
// cause an endless loop (see crbug.com/516070).
if (requests_.empty()) { if (requests_.empty()) {
requests_.push_back(GaiaCookieRequest::CreateListAccountsRequest()); requests_.push_back(GaiaCookieRequest::CreateListAccountsRequest());
fetcher_retries_ = 0; fetcher_retries_ = 0;
...@@ -686,6 +682,17 @@ void GaiaCookieManagerService::OnCookieChange( ...@@ -686,6 +682,17 @@ void GaiaCookieManagerService::OnCookieChange(
signin_client_->DelayNetworkCall( signin_client_->DelayNetworkCall(
base::BindOnce(&GaiaCookieManagerService::StartFetchingListAccounts, base::BindOnce(&GaiaCookieManagerService::StartFetchingListAccounts,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
} else {
// Remove all /ListAccounts requests except the very first request because
// it is currently executing.
requests_.erase(std::remove_if(requests_.begin() + 1, requests_.end(),
[](const GaiaCookieRequest& request) {
return request.request_type() ==
LIST_ACCOUNTS;
}),
requests_.end());
// Add a new /ListAccounts request at the end.
requests_.push_back(GaiaCookieRequest::CreateListAccountsRequest());
} }
} }
......
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