Commit bed26fe6 authored by Sylvain Defresne's avatar Sylvain Defresne Committed by Commit Bot

Fix confusing code that appeared to be causing UB

By the time ResetChromeIdentityServiceObserverForTesting() is
called, the old ChromeIdentityService has been destroyed thus
it is Undefined Behaviour to call RemoveObserver() via the
ScopedObserver::RemoveAll() method.

The code does not exhibit Undefined Behaviour because the
destructor of ChromeIdentityService notify all its observer
(via OnChromeIdentityServiceWillBeDestroyed) and they remove
themselves from the observer list.

Instead change ResetChromeIdentityServiceObserverForTesting()
to DCHECK() that the call to RemoveAll() has been performed
(via the OnChromeIdentityServiceWillBeDestroyed implementation)
by checking the ScopedObserver is not registered in any list.

Bug: 957973
Change-Id: Iccd019405f4972470fddbd9b7540232cc9bb3946
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1589955Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655928}
parent 65c34e80
......@@ -424,7 +424,7 @@ bool AuthenticationService::ShowMDMErrorDialogForIdentity(
}
void AuthenticationService::ResetChromeIdentityServiceObserverForTesting() {
identity_service_observer_.RemoveAll();
DCHECK(!identity_service_observer_.IsObservingSources());
identity_service_observer_.Add(
ios::GetChromeBrowserProvider()->GetChromeIdentityService());
}
......
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