GitLab

It is incorrect to cache the value of ArcAppListPrefs::Get(profile). The
value returned can change over the lifetime of the profile.

Also move observer clean-up from the ArcApps destructor to its
KeyedService::Shutdown method, since the ArcApps keyed service depends
on the AALP keyed service.

This fixes the ArcAppLauncherForDefaultAppTest.* unit_tests in debug
mode, when combined with --enable-features=AppServiceAsh. The tests
passed in release mode, both before and after this commit.

In the unit_tests, ArcAppModelBuilderRecreate::StartArc calls
ArcAppListPrefsFactory::GetInstance()->RecreateServiceInstanceForTesting,
which destroys the existing ArcAppListPrefs and creates a new one.
Meanwhile, ArcApps code continues to follow the old pointer.

In the release build, there's the (un)happy coincidence that the new
AALP's pointer value is exactly the same as the old one: the memory is
re-used and that 'dangling' pointer still points to a valid AALP object.
In the debug build, malloc/free behavior is presumably different (the
pointer value is certainly different), and the latent bug surfaces.

BUG=826982

Change-Id: Ie77fb2ab8c65662d66faff8947901414d9cb1892
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1732110
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683475}