Commit 2e98d5f6 authored by Jeroen Dhollander's avatar Jeroen Dhollander Committed by Commit Bot

Do not store pointer to AssistantInteractionController

During shutdown this pointer might become dangling.
Instead, we fetch the value through |AssistantInteractionController::Get()|
whenever we need it (and check to ensure it is not nullptr).

I also checked if we need to protect the |IdentityManager*|, but that
should be safe as |BloomController| is created together with |Service|
(*), which also contains a pointer to |IdentityManager|.

(*) Here: http://shortn/_l71GCzVgX3

Bug: b/165356952
Tests: compiled
Change-Id: I9e1e9ed83db9a42221e1c2f0e0f3dbe14a33e0c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2372591
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803941}
parent e285bf93
......@@ -92,8 +92,7 @@ void AssistantClientImpl::MaybeInit(Profile* profile) {
if (chromeos::assistant::features::IsBloomEnabled()) {
bloom_controller_ = chromeos::bloom::BloomControllerFactory::Create(
profile->GetURLLoaderFactory(),
IdentityManagerFactory::GetForProfile(profile),
ash::AssistantInteractionController::Get());
IdentityManagerFactory::GetForProfile(profile));
}
}
......
......@@ -9,16 +9,13 @@
namespace chromeos {
namespace bloom {
BloomInteractionObserverImpl::BloomInteractionObserverImpl(
ash::AssistantInteractionController* assistant_interaction_controller)
: assistant_interaction_controller_(assistant_interaction_controller) {
DCHECK(assistant_interaction_controller_);
}
BloomInteractionObserverImpl::BloomInteractionObserverImpl() = default;
BloomInteractionObserverImpl::~BloomInteractionObserverImpl() = default;
void BloomInteractionObserverImpl::OnInteractionStarted() {
assistant_interaction_controller_->StartBloomInteraction();
if (assistant_interaction_controller())
assistant_interaction_controller()->StartBloomInteraction();
}
void BloomInteractionObserverImpl::OnShowUI() {
......@@ -34,5 +31,10 @@ void BloomInteractionObserverImpl::OnInteractionFinished(
// TODO(jeroendh): implement
}
ash::AssistantInteractionController*
BloomInteractionObserverImpl::assistant_interaction_controller() {
return ash::AssistantInteractionController::Get();
}
} // namespace bloom
} // namespace chromeos
......@@ -19,8 +19,7 @@ namespace bloom {
// all bloom responses.
class BloomInteractionObserverImpl : public BloomInteractionObserver {
public:
explicit BloomInteractionObserverImpl(
ash::AssistantInteractionController* assistant_interaction_controller);
BloomInteractionObserverImpl();
~BloomInteractionObserverImpl() override;
// BloomInteractionObserver implementation:
......@@ -30,7 +29,9 @@ class BloomInteractionObserverImpl : public BloomInteractionObserver {
void OnInteractionFinished(BloomInteractionResolution resolution) override;
private:
ash::AssistantInteractionController* assistant_interaction_controller_;
// Return the current |AssistantInteractionController|. Can be nullptr
// (usually if the system is in the process of shutting down/logging out).
ash::AssistantInteractionController* assistant_interaction_controller();
};
} // namespace bloom
......
......@@ -28,14 +28,12 @@ class FakeScreenshotGrabber : public ScreenshotGrabber {
// static
std::unique_ptr<BloomController> BloomControllerFactory::Create(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
signin::IdentityManager* identity_manager,
ash::AssistantInteractionController* assistant_interaction_controller) {
signin::IdentityManager* identity_manager) {
auto result = std::make_unique<BloomControllerImpl>(
identity_manager, std::make_unique<FakeScreenshotGrabber>(),
std::make_unique<BloomServerProxyImpl>());
result->AddObserver(std::make_unique<BloomInteractionObserverImpl>(
assistant_interaction_controller));
result->AddObserver(std::make_unique<BloomInteractionObserverImpl>());
return std::move(result);
}
......
......@@ -10,10 +10,6 @@
#include "base/component_export.h"
#include "base/memory/scoped_refptr.h"
namespace ash {
class AssistantInteractionController;
} // namespace ash
namespace network {
class SharedURLLoaderFactory;
} // namespace network
......@@ -32,8 +28,7 @@ class COMPONENT_EXPORT(BLOOM) BloomControllerFactory {
// Create the Bloom controller. Can only be invoked once.
static std::unique_ptr<BloomController> Create(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
signin::IdentityManager* identity_manager,
ash::AssistantInteractionController* assistant_interaction_controller);
signin::IdentityManager* identity_manager);
};
} // namespace bloom
......
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