Commit e441cd0c authored by David Black's avatar David Black Committed by Commit Bot

Migrate Assistant cards off of Content Service.

Manual tests run:
- Tab traverse forwards
- Tab traverse backwards
- ChromeVox traverse forwards
- ChromeVox traverse backwards
- Click inside non-navigable card
- Tap inside non-navigable card
- Click inside navigable card
- Tap inside navigable card
- Tab traverse multiple cards

Bug: b:146351046
Change-Id: I72bb661765792ec0d222c39dd82df3f0795d6d81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003219Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#732583}
parent 9a358448
...@@ -792,15 +792,8 @@ void AssistantInteractionController::OnProcessPendingResponse() { ...@@ -792,15 +792,8 @@ void AssistantInteractionController::OnProcessPendingResponse() {
return; return;
} }
// Bind an interface to a navigable contents factory that is needed for
// processing card elements.
mojo::Remote<content::mojom::NavigableContentsFactory> factory;
assistant_controller_->GetNavigableContentsFactory(
factory.BindNewPipeAndPassReceiver());
// Start processing. // Start processing.
model_.pending_response()->Process( model_.pending_response()->Process(
std::move(factory),
base::BindOnce( base::BindOnce(
&AssistantInteractionController::OnPendingResponseProcessed, &AssistantInteractionController::OnPendingResponseProcessed,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
......
...@@ -55,11 +55,8 @@ AssistantResponse::GetSuggestions() const { ...@@ -55,11 +55,8 @@ AssistantResponse::GetSuggestions() const {
return suggestions; return suggestions;
} }
void AssistantResponse::Process( void AssistantResponse::Process(ProcessingCallback callback) {
mojo::Remote<content::mojom::NavigableContentsFactory> contents_factory, processor_ = std::make_unique<Processor>(*this, std::move(callback));
ProcessingCallback callback) {
processor_ = std::make_unique<Processor>(*this, std::move(contents_factory),
std::move(callback));
processor_->Process(); processor_->Process();
} }
...@@ -67,10 +64,8 @@ void AssistantResponse::Process( ...@@ -67,10 +64,8 @@ void AssistantResponse::Process(
AssistantResponse::Processor::Processor( AssistantResponse::Processor::Processor(
AssistantResponse& response, AssistantResponse& response,
mojo::Remote<content::mojom::NavigableContentsFactory> contents_factory,
ProcessingCallback callback) ProcessingCallback callback)
: response_(response), : response_(response),
contents_factory_(std::move(contents_factory)),
callback_(std::move(callback)) {} callback_(std::move(callback)) {}
AssistantResponse::Processor::~Processor() { AssistantResponse::Processor::~Processor() {
...@@ -89,10 +84,9 @@ void AssistantResponse::Processor::Process() { ...@@ -89,10 +84,9 @@ void AssistantResponse::Processor::Process() {
++processing_count_; ++processing_count_;
// Start asynchronous processing of the card element. // Start asynchronous processing of the card element.
static_cast<AssistantCardElement*>(ui_element.get()) static_cast<AssistantCardElement*>(ui_element.get())
->Process(contents_factory_.get(), ->Process(base::BindOnce(
base::BindOnce( &AssistantResponse::Processor::OnFinishedProcessing,
&AssistantResponse::Processor::OnFinishedProcessing, base::Unretained(this)));
base::Unretained(this)));
break; break;
case AssistantUiElementType::kText: case AssistantUiElementType::kText:
// No processing necessary. // No processing necessary.
......
...@@ -9,12 +9,11 @@ ...@@ -9,12 +9,11 @@
#include <memory> #include <memory>
#include <vector> #include <vector>
#include "base/callback.h"
#include "base/component_export.h" #include "base/component_export.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "chromeos/services/assistant/public/mojom/assistant.mojom-forward.h" #include "chromeos/services/assistant/public/mojom/assistant.mojom-forward.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/content/public/cpp/navigable_contents.h"
namespace ash { namespace ash {
...@@ -70,9 +69,7 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponse ...@@ -70,9 +69,7 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponse
// Invoke to begin processing the response. Upon completion, |callback| will // Invoke to begin processing the response. Upon completion, |callback| will
// be run to indicate success or failure. // be run to indicate success or failure.
void Process( void Process(ProcessingCallback callback);
mojo::Remote<content::mojom::NavigableContentsFactory> contents_factory,
ProcessingCallback callback);
private: private:
friend class base::RefCounted<AssistantResponse>; friend class base::RefCounted<AssistantResponse>;
...@@ -83,7 +80,6 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponse ...@@ -83,7 +80,6 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponse
public: public:
Processor( Processor(
AssistantResponse& response, AssistantResponse& response,
mojo::Remote<content::mojom::NavigableContentsFactory> contents_factory,
ProcessingCallback callback); ProcessingCallback callback);
~Processor(); ~Processor();
...@@ -101,7 +97,6 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponse ...@@ -101,7 +97,6 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponse
void TryFinishing(); void TryFinishing();
AssistantResponse& response_; AssistantResponse& response_;
mojo::Remote<content::mojom::NavigableContentsFactory> contents_factory_;
ProcessingCallback callback_; ProcessingCallback callback_;
int processing_count_ = 0; int processing_count_ = 0;
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "ash/assistant/model/ui/assistant_card_element.h" #include "ash/assistant/model/ui/assistant_card_element.h"
#include "ash/assistant/ui/assistant_ui_constants.h" #include "ash/assistant/ui/assistant_ui_constants.h"
#include "ash/public/cpp/assistant/assistant_web_view_factory.h"
#include "base/base64.h" #include "base/base64.h"
namespace ash { namespace ash {
...@@ -19,11 +20,8 @@ AssistantCardElement::AssistantCardElement(const std::string& html, ...@@ -19,11 +20,8 @@ AssistantCardElement::AssistantCardElement(const std::string& html,
AssistantCardElement::~AssistantCardElement() = default; AssistantCardElement::~AssistantCardElement() = default;
void AssistantCardElement::Process( void AssistantCardElement::Process(ProcessingCallback callback) {
content::mojom::NavigableContentsFactory* contents_factory, processor_ = std::make_unique<Processor>(*this, std::move(callback));
ProcessingCallback callback) {
processor_ =
std::make_unique<Processor>(*this, contents_factory, std::move(callback));
processor_->Process(); processor_->Process();
} }
...@@ -31,15 +29,13 @@ void AssistantCardElement::Process( ...@@ -31,15 +29,13 @@ void AssistantCardElement::Process(
AssistantCardElement::Processor::Processor( AssistantCardElement::Processor::Processor(
AssistantCardElement& card_element, AssistantCardElement& card_element,
content::mojom::NavigableContentsFactory* contents_factory,
ProcessingCallback callback) ProcessingCallback callback)
: card_element_(card_element), : card_element_(card_element),
contents_factory_(contents_factory),
callback_(std::move(callback)) {} callback_(std::move(callback)) {}
AssistantCardElement::Processor::~Processor() { AssistantCardElement::Processor::~Processor() {
if (contents_) if (contents_view_)
contents_->RemoveObserver(this); contents_view_->RemoveObserver(this);
if (callback_) if (callback_)
std::move(callback_).Run(/*success=*/false); std::move(callback_).Run(/*success=*/false);
...@@ -50,17 +46,19 @@ void AssistantCardElement::Processor::Process() { ...@@ -50,17 +46,19 @@ void AssistantCardElement::Processor::Process() {
const int width_dip = kPreferredWidthDip - 2 * kUiElementHorizontalMarginDip; const int width_dip = kPreferredWidthDip - 2 * kUiElementHorizontalMarginDip;
// Configure parameters for the card. // Configure parameters for the card.
auto contents_params = content::mojom::NavigableContentsParams::New(); AssistantWebView2::InitParams contents_params;
contents_params->enable_view_auto_resize = true; contents_params.enable_auto_resize = true;
contents_params->auto_resize_min_size = gfx::Size(width_dip, 1); contents_params.min_size = gfx::Size(width_dip, 1);
contents_params->auto_resize_max_size = gfx::Size(width_dip, INT_MAX); contents_params.max_size = gfx::Size(width_dip, INT_MAX);
contents_params->suppress_navigations = true; contents_params.suppress_navigation = true;
contents_ = std::make_unique<content::NavigableContents>( // Create |contents_view_| and retain ownership so that is properly cleaned up
contents_factory_, std::move(contents_params)); // in the case where it is never added to the view hierarchy.
contents_view_ = AssistantWebViewFactory::Get()->Create(contents_params);
contents_view_->set_owned_by_client();
// Observe |contents_| so that we are notified when loading is complete. // Observe |contents_view_| so that we are notified when loading is complete.
contents_->AddObserver(this); contents_view_->AddObserver(this);
// Encode the html string to be URL-safe. // Encode the html string to be URL-safe.
std::string encoded_html; std::string encoded_html;
...@@ -68,15 +66,15 @@ void AssistantCardElement::Processor::Process() { ...@@ -68,15 +66,15 @@ void AssistantCardElement::Processor::Process() {
// Navigate to the data URL which represents the card. // Navigate to the data URL which represents the card.
constexpr char kDataUriPrefix[] = "data:text/html;base64,"; constexpr char kDataUriPrefix[] = "data:text/html;base64,";
contents_->Navigate(GURL(kDataUriPrefix + encoded_html)); contents_view_->Navigate(GURL(kDataUriPrefix + encoded_html));
} }
void AssistantCardElement::Processor::DidStopLoading() { void AssistantCardElement::Processor::DidStopLoading() {
contents_->RemoveObserver(this); contents_view_->RemoveObserver(this);
// Pass ownership of |contents_| to the card element that was being processed // Pass ownership of |contents_view_| to the card element that was being
// and notify our |callback_| of success. // processed and notify our |callback_| of success.
card_element_.set_contents(std::move(contents_)); card_element_.set_contents_view(std::move(contents_view_));
std::move(callback_).Run(/*success=*/true); std::move(callback_).Run(/*success=*/true);
} }
......
...@@ -9,9 +9,9 @@ ...@@ -9,9 +9,9 @@
#include <string> #include <string>
#include "ash/assistant/model/ui/assistant_ui_element.h" #include "ash/assistant/model/ui/assistant_ui_element.h"
#include "ash/public/cpp/assistant/assistant_web_view_2.h"
#include "base/component_export.h" #include "base/component_export.h"
#include "base/macros.h" #include "base/macros.h"
#include "services/content/public/cpp/navigable_contents.h"
namespace ash { namespace ash {
...@@ -29,45 +29,44 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantCardElement ...@@ -29,45 +29,44 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantCardElement
const std::string& html() const { return html_; } const std::string& html() const { return html_; }
const std::string& fallback() const { return fallback_; } const std::string& fallback() const { return fallback_; }
const content::NavigableContents* contents() const { return contents_.get(); }
void set_contents(std::unique_ptr<content::NavigableContents> contents) { const AssistantWebView2* contents_view() const {
contents_ = std::move(contents); return contents_view_.get();
}
void set_contents_view(std::unique_ptr<AssistantWebView2> contents_view) {
contents_view_ = std::move(contents_view);
} }
// Invoke to begin processing the card element. Upon completion, the specified // Invoke to begin processing the card element. Upon completion, the specified
// |callback| will be run to indicate success or failure. // |callback| will be run to indicate success or failure.
void Process(content::mojom::NavigableContentsFactory* contents_factory, void Process(ProcessingCallback callback);
ProcessingCallback callback);
private: private:
// Handles processing of an AssistantCardElement. // Handles processing of an AssistantCardElement.
class Processor : public content::NavigableContentsObserver { class Processor : public AssistantWebView2::Observer {
public: public:
Processor(AssistantCardElement& card_element, Processor(AssistantCardElement& card_element, ProcessingCallback callback);
content::mojom::NavigableContentsFactory* contents_factory,
ProcessingCallback callback);
~Processor() override; ~Processor() override;
// Invoke to begin processing. // Invoke to begin processing.
void Process(); void Process();
// content::NavigableContentsObserver: // AssistantWebView2::Observer:
void DidStopLoading() override; void DidStopLoading() override;
private: private:
AssistantCardElement& card_element_; AssistantCardElement& card_element_;
content::mojom::NavigableContentsFactory* const contents_factory_;
ProcessingCallback callback_; ProcessingCallback callback_;
std::unique_ptr<content::NavigableContents> contents_; std::unique_ptr<AssistantWebView2> contents_view_;
DISALLOW_COPY_AND_ASSIGN(Processor); DISALLOW_COPY_AND_ASSIGN(Processor);
}; };
const std::string html_; const std::string html_;
const std::string fallback_; const std::string fallback_;
std::unique_ptr<content::NavigableContents> contents_; std::unique_ptr<AssistantWebView2> contents_view_;
std::unique_ptr<Processor> processor_; std::unique_ptr<Processor> processor_;
......
...@@ -61,13 +61,13 @@ AssistantCardElementView::AssistantCardElementView( ...@@ -61,13 +61,13 @@ AssistantCardElementView::AssistantCardElementView(
: delegate_(delegate), card_element_(card_element) { : delegate_(delegate), card_element_(card_element) {
InitLayout(card_element); InitLayout(card_element);
// We observe contents() to receive events pertaining to the underlying web // We observe contents_view() to receive events pertaining to the underlying
// contents including auto-resize and suppressed navigation events. // WebContents including focus change and suppressed navigation events.
contents()->AddObserver(this); contents_view()->AddObserver(this);
} }
AssistantCardElementView::~AssistantCardElementView() { AssistantCardElementView::~AssistantCardElementView() {
contents()->RemoveObserver(this); contents_view()->RemoveObserver(this);
} }
const char* AssistantCardElementView::GetClassName() const { const char* AssistantCardElementView::GetClassName() const {
...@@ -102,17 +102,6 @@ void AssistantCardElementView::ChildPreferredSizeChanged(views::View* child) { ...@@ -102,17 +102,6 @@ void AssistantCardElementView::ChildPreferredSizeChanged(views::View* child) {
PreferredSizeChanged(); PreferredSizeChanged();
} }
void AssistantCardElementView::AboutToRequestFocusFromTabTraversal(
bool reverse) {
// Focus in the web contents will be reset in FocusThroughTabTraversal().
focused_node_rect_ = gfx::Rect();
contents()->FocusThroughTabTraversal(reverse);
}
void AssistantCardElementView::OnFocus() {
contents()->Focus();
}
void AssistantCardElementView::OnGestureEvent(ui::GestureEvent* event) { void AssistantCardElementView::OnGestureEvent(ui::GestureEvent* event) {
// We need to route GESTURE_TAP events to our Assistant card because links // We need to route GESTURE_TAP events to our Assistant card because links
// should be tappable. The Assistant card window will not receive gesture // should be tappable. The Assistant card window will not receive gesture
...@@ -174,10 +163,6 @@ void AssistantCardElementView::ScrollRectToVisible(const gfx::Rect& rect) { ...@@ -174,10 +163,6 @@ void AssistantCardElementView::ScrollRectToVisible(const gfx::Rect& rect) {
views::View::ScrollRectToVisible(focused_node_rect_); views::View::ScrollRectToVisible(focused_node_rect_);
} }
void AssistantCardElementView::DidAutoResizeView(const gfx::Size& new_size) {
contents()->GetView()->view()->SetPreferredSize(new_size);
}
void AssistantCardElementView::DidSuppressNavigation( void AssistantCardElementView::DidSuppressNavigation(
const GURL& url, const GURL& url,
WindowOpenDisposition disposition, WindowOpenDisposition disposition,
...@@ -205,8 +190,7 @@ void AssistantCardElementView::DidSuppressNavigation( ...@@ -205,8 +190,7 @@ void AssistantCardElementView::DidSuppressNavigation(
delegate_->OpenUrlFromView(url); delegate_->OpenUrlFromView(url);
} }
void AssistantCardElementView::FocusedNodeChanged( void AssistantCardElementView::DidChangeFocusedNode(
bool is_editable_node,
const gfx::Rect& node_bounds_in_screen) { const gfx::Rect& node_bounds_in_screen) {
// TODO(b/143985066): Card has element with empty bounds, e.g. the line break. // TODO(b/143985066): Card has element with empty bounds, e.g. the line break.
if (node_bounds_in_screen.IsEmpty()) if (node_bounds_in_screen.IsEmpty())
...@@ -224,14 +208,14 @@ void AssistantCardElementView::InitLayout( ...@@ -224,14 +208,14 @@ void AssistantCardElementView::InitLayout(
SetLayoutManager(std::make_unique<views::FillLayout>()); SetLayoutManager(std::make_unique<views::FillLayout>());
// Contents view. // Contents view.
AddChildView(contents()->GetView()->view()); AddChildView(contents_view());
// OverrideDescription() doesn't work. Only names are read automatically. // OverrideDescription() doesn't work. Only names are read automatically.
GetViewAccessibility().OverrideName(card_element->fallback()); GetViewAccessibility().OverrideName(card_element->fallback());
} }
content::NavigableContents* AssistantCardElementView::contents() { AssistantWebView2* AssistantCardElementView::contents_view() {
return const_cast<content::NavigableContents*>(card_element_->contents()); return const_cast<AssistantWebView2*>(card_element_->contents_view());
} }
} // namespace ash } // namespace ash
...@@ -8,10 +8,9 @@ ...@@ -8,10 +8,9 @@
#include <string> #include <string>
#include "ash/assistant/ui/main_stage/assistant_ui_element_view.h" #include "ash/assistant/ui/main_stage/assistant_ui_element_view.h"
#include "ash/public/cpp/assistant/assistant_web_view_2.h"
#include "base/component_export.h" #include "base/component_export.h"
#include "base/macros.h" #include "base/macros.h"
#include "services/content/public/cpp/navigable_contents.h"
#include "services/content/public/cpp/navigable_contents_view.h"
namespace ash { namespace ash {
...@@ -22,7 +21,7 @@ class AssistantViewDelegate; ...@@ -22,7 +21,7 @@ class AssistantViewDelegate;
// AssistantCardElement. It is a child view of UiElementContainerView. // AssistantCardElement. It is a child view of UiElementContainerView.
class COMPONENT_EXPORT(ASSISTANT_UI) AssistantCardElementView class COMPONENT_EXPORT(ASSISTANT_UI) AssistantCardElementView
: public AssistantUiElementView, : public AssistantUiElementView,
public content::NavigableContentsObserver { public AssistantWebView2::Observer {
public: public:
AssistantCardElementView(AssistantViewDelegate* delegate, AssistantCardElementView(AssistantViewDelegate* delegate,
const AssistantCardElement* card_element); const AssistantCardElement* card_element);
...@@ -34,34 +33,30 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantCardElementView ...@@ -34,34 +33,30 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantCardElementView
std::string ToStringForTesting() const override; std::string ToStringForTesting() const override;
void AddedToWidget() override; void AddedToWidget() override;
void ChildPreferredSizeChanged(views::View* child) override; void ChildPreferredSizeChanged(views::View* child) override;
void AboutToRequestFocusFromTabTraversal(bool reverse) override;
void OnFocus() override;
void OnGestureEvent(ui::GestureEvent* event) override; void OnGestureEvent(ui::GestureEvent* event) override;
void ScrollRectToVisible(const gfx::Rect& rect) override; void ScrollRectToVisible(const gfx::Rect& rect) override;
// content::NavigableContentsObserver: // AssistantWebView2::Observer:
void DidAutoResizeView(const gfx::Size& new_size) override;
void DidSuppressNavigation(const GURL& url, void DidSuppressNavigation(const GURL& url,
WindowOpenDisposition disposition, WindowOpenDisposition disposition,
bool from_user_gesture) override; bool from_user_gesture) override;
void FocusedNodeChanged(bool is_editable_node, void DidChangeFocusedNode(const gfx::Rect& node_bounds_in_screen) override;
const gfx::Rect& node_bounds_in_screen) override;
// Returns a reference to the native view associated with the underlying web // Returns a reference to the native view associated with the underlying web
// contents. When animating AssistantCardElementView, we should animate the // contents. When animating AssistantCardElementView, we should animate the
// layer for the native view as opposed to painting to and animating a layer // layer for the native view as opposed to painting to and animating a layer
// belonging to AssistantCardElementView. // belonging to AssistantCardElementView.
gfx::NativeView native_view() { return contents()->GetView()->native_view(); } gfx::NativeView native_view() { return contents_view()->GetNativeView(); }
private: private:
void InitLayout(const AssistantCardElement* card_element); void InitLayout(const AssistantCardElement* card_element);
content::NavigableContents* contents(); AssistantWebView2* contents_view();
AssistantViewDelegate* const delegate_; AssistantViewDelegate* const delegate_;
const AssistantCardElement* const card_element_; const AssistantCardElement* const card_element_;
// Rect of the focused node in the |contents_|. // Rect of the focused node in the |contents_view()|.
gfx::Rect focused_node_rect_; gfx::Rect focused_node_rect_;
DISALLOW_COPY_AND_ASSIGN(AssistantCardElementView); DISALLOW_COPY_AND_ASSIGN(AssistantCardElementView);
......
...@@ -130,7 +130,7 @@ void UiElementContainerView::HandleResponse(const AssistantResponse& response) { ...@@ -130,7 +130,7 @@ void UiElementContainerView::HandleResponse(const AssistantResponse& response) {
void UiElementContainerView::OnCardElementAdded( void UiElementContainerView::OnCardElementAdded(
const AssistantCardElement* card_element) { const AssistantCardElement* card_element) {
// The card, for some reason, is not embeddable so we'll have to ignore it. // The card, for some reason, is not embeddable so we'll have to ignore it.
if (!card_element->contents()) if (!card_element->contents_view())
return; return;
auto* card_element_view = auto* card_element_view =
......
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