Skip to content

Commit

Permalink
[M101-merge] assistant: Speculative fix of card element view
Browse files Browse the repository at this point in the history
We suspect that when ui_element is added, the contents_view is nullptr.
Add a check before creating the card element view.
We still need to investigate why the ui_element could be added multiple
times or why the contents_view is nullptr. Will track in b/228109139

Bug: 1303227
Test: NA

(cherry picked from commit f996270)

Change-Id: I573418dc04567756362bb765e2715ea7b0e41530
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3573689
Reviewed-by: Eric Sum <esum@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#989684}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3588959
Cr-Commit-Position: refs/branch-heads/4951@{#791}
Cr-Branched-From: 27de622-refs/heads/main@{#982481}
  • Loading branch information
wutao authored and Chromium LUCI CQ committed Apr 16, 2022
1 parent c72f07a commit 5e1b9fc
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 3 deletions.
4 changes: 4 additions & 0 deletions ash/assistant/model/ui/assistant_card_element.cc
Expand Up @@ -95,6 +95,10 @@ void AssistantCardElement::Process(ProcessingCallback callback) {
processor_->Process();
}

bool AssistantCardElement::has_contents_view() const {
return !!contents_view_;
}

bool AssistantCardElement::Compare(const AssistantUiElement& other) const {
return other.type() == AssistantUiElementType::kCard &&
static_cast<const AssistantCardElement&>(other).html() == html_;
Expand Down
1 change: 1 addition & 0 deletions ash/assistant/model/ui/assistant_card_element.h
Expand Up @@ -30,6 +30,7 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantCardElement
// AssistantUiElement:
void Process(ProcessingCallback callback) override;

bool has_contents_view() const;
const std::string& html() const { return html_; }
const std::string& fallback() const { return fallback_; }
std::unique_ptr<AshWebView> MoveContentsView() {
Expand Down
15 changes: 12 additions & 3 deletions ash/assistant/ui/main_stage/assistant_ui_element_view_factory.cc
Expand Up @@ -13,6 +13,7 @@
#include "ash/assistant/ui/main_stage/assistant_error_element_view.h"
#include "ash/assistant/ui/main_stage/assistant_text_element_view.h"
#include "ash/assistant/ui/main_stage/assistant_ui_element_view.h"
#include "base/logging.h"

namespace ash {

Expand All @@ -25,9 +26,17 @@ AssistantUiElementViewFactory::~AssistantUiElementViewFactory() = default;
std::unique_ptr<AssistantUiElementView> AssistantUiElementViewFactory::Create(
const AssistantUiElement* ui_element) const {
switch (ui_element->type()) {
case AssistantUiElementType::kCard:
return std::make_unique<AssistantCardElementView>(
delegate_, static_cast<const AssistantCardElement*>(ui_element));
case AssistantUiElementType::kCard: {
auto* card_element = static_cast<const AssistantCardElement*>(ui_element);
if (!card_element->has_contents_view()) {
// TODO(b/228109139): Find the root cause why reaches here.
LOG(DFATAL) << "AssistantCardElement has null contents_view. Not "
"create the view.";
return nullptr;
}
return std::make_unique<AssistantCardElementView>(delegate_,
card_element);
}
case AssistantUiElementType::kError:
return std::make_unique<AssistantErrorElementView>(
static_cast<const AssistantErrorElement*>(ui_element));
Expand Down
3 changes: 3 additions & 0 deletions ash/assistant/ui/main_stage/ui_element_container_view.cc
Expand Up @@ -206,6 +206,9 @@ std::unique_ptr<ElementAnimator> UiElementContainerView::HandleUiElement(
const AssistantUiElement* ui_element) {
// Create a new view for the |ui_element|.
auto view = view_factory_->Create(ui_element);
if (!view) {
return nullptr;
}

// If the first UI element is a card, it has a unique margin requirement.
const bool is_card = ui_element->type() == AssistantUiElementType::kCard;
Expand Down

0 comments on commit 5e1b9fc

Please sign in to comment.