Use a token instead of view in SearchResult

Use a token to represent answer card contents since SearchResult data
will go through mojo and views::View* will not work. When app list
search code is in the same process of its ui, AnswerCardCotnentsRegistry
works as a middle man to map token to underlying view. This CL covers
this part. When they are in two processes, the token is the embedding
token that could be used to embed answer card contents in app list ui.
This part will be implemented in a follow-up CL.

Bug: 812434
Change-Id: Ic15bb89c40e370bb3c5967f38efa58828be3967f
Reviewed-on: https://chromium-review.googlesource.com/957742Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543368}

ash/app_list/app_list_controller_impl.cc

@@ -18,6 +18,7 @@
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "extensions/common/constants.h"
+#include "ui/app_list/answer_card_contents_registry.h"
 #include "ui/app_list/views/app_list_main_view.h"
 #include "ui/app_list/views/app_list_view.h"
 #include "ui/app_list/views/contents_view.h"
@@ -59,6 +60,13 @@ AppListControllerImpl::AppListControllerImpl()
               std::make_unique<ViewDelegateFactoryImpl>(&view_delegate_)),
           this) {
   model_.AddObserver(this);
+
+  // Create only for non-mash. Mash uses window tree embed API to get a
+  // token to map answer card contents.
+  if (Shell::GetAshConfig() != Config::MASH) {
+    answer_card_contents_registry_ =
+        std::make_unique<app_list::AnswerCardContentsRegistry>();
+  }
 }
 
 AppListControllerImpl::~AppListControllerImpl() {

ash/app_list/app_list_controller_impl.h

@@ -26,6 +26,10 @@ namespace ui {
 class MouseWheelEvent;
 }  // namespace ui
 
+namespace app_list {
+class AnswerCardContentsRegistry;
+}  // namespace app_list
+
 namespace ash {
 
 // Ash's AppListController owns the AppListModel and implements interface
@@ -145,6 +149,10 @@ class ASH_EXPORT AppListControllerImpl : public mojom::AppListController,
 
   mojom::AppListClientPtr client_;
 
+  // Token to view map for classic/mus ash (i.e. non-mash).
+  std::unique_ptr<app_list::AnswerCardContentsRegistry>
+      answer_card_contents_registry_;
+
   DISALLOW_COPY_AND_ASSIGN(AppListControllerImpl);
 };
 

ash/app_list/model/search/search_model.cc

@@ -70,7 +70,8 @@ void SearchModel::PublishResults(
   for (auto&& new_result : new_results) {
     auto ui_result_it = results_map.find(new_result->id());
     if (ui_result_it != results_map.end() &&
-        new_result->view() == ui_result_it->second->view()) {
+        new_result->answer_card_contents_token() ==
+            ui_result_it->second->answer_card_contents_token()) {
       // Update and use the old result if it exists.
       std::unique_ptr<SearchResult> ui_result = std::move(ui_result_it->second);
       UpdateResult(new_result.get(), ui_result.get());

ash/app_list/model/search/search_result.h

@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/strings/string16.h"
+#include "base/unguessable_token.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/range/range.h"
 
@@ -22,10 +23,6 @@ namespace ui {
 class MenuModel;
 }
 
-namespace views {
-class View;
-}
-
 namespace app_list {
 
 class SearchResultObserver;
@@ -130,8 +127,12 @@ class APP_LIST_MODEL_EXPORT SearchResult {
   const base::string16& formatted_price() const { return formatted_price_; }
   void SetFormattedPrice(const base::string16& formatted_price);
 
-  views::View* view() const { return view_; }
-  void set_view(views::View* view) { view_ = view; }
+  const base::UnguessableToken& answer_card_contents_token() const {
+    return answer_card_contents_token_;
+  }
+  void set_answer_card_contents_token(const base::UnguessableToken& token) {
+    answer_card_contents_token_ = token;
+  }
 
   const std::string& id() const { return id_; }
   const std::string& comparable_id() const { return comparable_id_; }
@@ -225,11 +226,12 @@ class APP_LIST_MODEL_EXPORT SearchResult {
   // Formatted price label of the app in play store. Not exist if set to empty.
   base::string16 formatted_price_;
 
-  // Unowned pointer to a view containing a rendered result, or nullptr if there
-  // is no such view for the result.
-  // The view has set_owned_by_client() property set. It's a responsibility of
-  // SearchProvider to set this property and own this view.
-  views::View* view_ = nullptr;
+  // A token used to show answer card contents. When answer card provider and ui
+  // runs in the same process (i.e classic ash, or mash before UI migration),
+  // AnswerCardContensRegistry could be used to map the token to a view.
+  // Otherwise (mash after UI migration), the token is used to embed the answer
+  // card contents.
+  base::UnguessableToken answer_card_contents_token_;
 
   std::string id_;
   // ID that can be compared across results from different providers to remove

chrome/browser/ui/app_list/search/answer_card/answer_card_contents.h

@@ -6,7 +6,9 @@
 #define CHROME_BROWSER_UI_APP_LIST_SEARCH_ANSWER_CARD_ANSWER_CARD_CONTENTS_H_
 
 #include <string>
+
 #include "base/observer_list.h"
+#include "base/unguessable_token.h"
 
 class GURL;
 
@@ -14,10 +16,6 @@ namespace app_list {
 class AnswerCardResult;
 }
 
-namespace views {
-class View;
-}
-
 namespace app_list {
 
 // Abstract source of contents for AnswerCardSearchProvider.
@@ -49,8 +47,8 @@ class AnswerCardContents {
 
   // Loads contents from |url|.
   virtual void LoadURL(const GURL& url) = 0;
-  // Returns the view associated with the contents.
-  virtual views::View* GetView() = 0;
+  // Returns the token associated with the contents.
+  virtual const base::UnguessableToken& GetToken() const = 0;
 
   // Sets the delegate to process contents-related events.
   void SetDelegate(Delegate* delegate);

chrome/browser/ui/app_list/search/answer_card/answer_card_result.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/app_list/search/answer_card/answer_card_result.h"
 
+#include "base/unguessable_token.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/search/answer_card/answer_card_contents.h"
 #include "chrome/browser/ui/app_list/search/search_util.h"
@@ -24,7 +25,8 @@ AnswerCardResult::AnswerCardResult(Profile* profile,
   set_id(result_url);
   set_comparable_id(stripped_result_url);
   set_relevance(1);
-  set_view(contents ? contents->GetView() : nullptr);
+  set_answer_card_contents_token(contents ? contents->GetToken()
+                                          : base::UnguessableToken());
   set_title(result_title);
 
   if (contents)

chrome/browser/ui/app_list/search/answer_card/answer_card_result_unittest.cc

@@ -30,10 +30,10 @@ class AnswerCardTestContents : public AnswerCardContents {
 
   // AnswerCardContents overrides:
   void LoadURL(const GURL& url) override { NOTREACHED(); }
-  views::View* GetView() override { return &view_; }
+  const base::UnguessableToken& GetToken() const override { return token_; }
 
  private:
-  views::View view_;
+  base::UnguessableToken token_;
 
   DISALLOW_COPY_AND_ASSIGN(AnswerCardTestContents);
 };
@@ -59,7 +59,9 @@ class AnswerCardResultTest : public AppListTestBase {
     return app_list_controller_delegate_->last_opened_url();
   }
 
-  views::View* GetView() const { return contents_->GetView(); }
+  const base::UnguessableToken& GetToken() const {
+    return contents_->GetToken();
+  }
 
   // AppListTestBase overrides:
   void SetUp() override {
@@ -86,7 +88,7 @@ TEST_F(AnswerCardResultTest, Basic) {
   EXPECT_EQ(base::ASCIIToUTF16(kResultTitle), result->title());
   EXPECT_EQ(SearchResult::DISPLAY_CARD, result->display_type());
   EXPECT_EQ(1, result->relevance());
-  EXPECT_EQ(GetView(), result->view());
+  EXPECT_EQ(GetToken(), result->answer_card_contents_token());
 
   result->Open(ui::EF_NONE);
   EXPECT_EQ(kResultUrl, GetLastOpenedUrl().spec());
@@ -97,7 +99,7 @@ TEST_F(AnswerCardResultTest, Basic) {
   EXPECT_EQ(base::ASCIIToUTF16(kResultTitle), result1->title());
   EXPECT_EQ(SearchResult::DISPLAY_CARD, result1->display_type());
   EXPECT_EQ(1, result1->relevance());
-  EXPECT_EQ(GetView(), result1->view());
+  EXPECT_EQ(GetToken(), result1->answer_card_contents_token());
 }
 
 TEST_F(AnswerCardResultTest, NullContents) {

chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.cc

@@ -19,6 +19,7 @@
 #include "content/public/common/renderer_preferences.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
+#include "ui/app_list/answer_card_contents_registry.h"
 #include "ui/app_list/views/app_list_view.h"
 #include "ui/aura/window.h"
 #include "ui/views/controls/native/native_view_host.h"
@@ -159,9 +160,15 @@ AnswerCardWebContents::AnswerCardWebContents(Profile* profile)
   content::RenderViewHost* const rvh = web_contents_->GetRenderViewHost();
   if (rvh)
     AttachToHost(rvh->GetWidget());
+
+  if (AnswerCardContentsRegistry::Get())
+    token_ = AnswerCardContentsRegistry::Get()->Register(web_view_.get());
 }
 
 AnswerCardWebContents::~AnswerCardWebContents() {
+  if (AnswerCardContentsRegistry::Get() && !token_.is_empty())
+    AnswerCardContentsRegistry::Get()->Unregister(token_);
+
   DetachFromHost();
   web_contents_->SetDelegate(nullptr);
   Observe(nullptr);
@@ -177,8 +184,8 @@ void AnswerCardWebContents::LoadURL(const GURL& url) {
       gfx::Size(1, 1), gfx::Size(INT_MAX, INT_MAX));
 }
 
-views::View* AnswerCardWebContents::GetView() {
-  return web_view_.get();
+const base::UnguessableToken& AnswerCardWebContents::GetToken() const {
+  return token_;
 }
 
 void AnswerCardWebContents::ResizeDueToAutoResize(

chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/unguessable_token.h"
 #include "chrome/browser/ui/app_list/search/answer_card/answer_card_contents.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents_delegate.h"
@@ -30,7 +31,7 @@ class AnswerCardWebContents : public AnswerCardContents,
 
   // AnswerCardContents overrides:
   void LoadURL(const GURL& url) override;
-  views::View* GetView() override;
+  const base::UnguessableToken& GetToken() const override;
 
   // content::WebContentsDelegate overrides:
   void ResizeDueToAutoResize(content::WebContents* web_contents,
@@ -65,6 +66,8 @@ class AnswerCardWebContents : public AnswerCardContents,
 
   Profile* const profile_;  // Unowned
 
+  base::UnguessableToken token_;
+
   DISALLOW_COPY_AND_ASSIGN(AnswerCardWebContents);
 };
 

ui/app_list/BUILD.gn

@@ -9,6 +9,8 @@ assert(is_chromeos)
 
 component("app_list") {
   sources = [
+    "answer_card_contents_registry.cc",
+    "answer_card_contents_registry.h",
     "app_list_constants.cc",
     "app_list_constants.h",
     "app_list_export.h",

ui/app_list/answer_card_contents_registry.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/app_list/answer_card_contents_registry.h"
+
+#include "base/logging.h"
+
+namespace app_list {
+
+namespace {
+
+AnswerCardContentsRegistry* instance = nullptr;
+
+}  // namespace
+
+AnswerCardContentsRegistry::AnswerCardContentsRegistry() {
+  DCHECK(!instance);
+  instance = this;
+}
+
+AnswerCardContentsRegistry::~AnswerCardContentsRegistry() {
+  DCHECK_EQ(this, instance);
+  instance = nullptr;
+}
+
+// static
+AnswerCardContentsRegistry* AnswerCardContentsRegistry::Get() {
+  return instance;
+}
+
+base::UnguessableToken AnswerCardContentsRegistry::Register(
+    views::View* contents_view) {
+  const base::UnguessableToken token = base::UnguessableToken::Create();
+  contents_map_[token] = contents_view;
+  return token;
+}
+
+void AnswerCardContentsRegistry::Unregister(
+    const base::UnguessableToken& token) {
+  auto it = contents_map_.find(token);
+  if (it == contents_map_.end())
+    return;
+
+  contents_map_.erase(it);
+}
+
+views::View* AnswerCardContentsRegistry::GetView(
+    const base::UnguessableToken& token) {
+  auto it = contents_map_.find(token);
+  if (it == contents_map_.end())
+    return nullptr;
+
+  return it->second;
+}
+
+}  // namespace app_list

ui/app_list/answer_card_contents_registry.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_APP_LIST_ANSWER_CARD_CONTENTS_REGISTRY_H_
+#define UI_APP_LIST_ANSWER_CARD_CONTENTS_REGISTRY_H_
+
+#include <map>
+
+#include "base/macros.h"
+#include "base/unguessable_token.h"
+#include "ui/app_list/app_list_export.h"
+
+namespace views {
+class View;
+}
+
+namespace app_list {
+
+// Helper class to provide a token to answer card view map when app list UI code
+// runs in the same process of answer card provider (classic ash, or mash before
+// UI code is migrated to ash). In such environment, chrome creates and owns an
+// instance of this class. AnswerCardContents registers answer card contents
+// with it and get a token back. App list UI code gets the token and use it to
+// look up the view to show. On mash after the UI code migration, this class
+// will NOT be used at all. App list UI code would use the token to embed the
+// answer card content directly.
+class APP_LIST_EXPORT AnswerCardContentsRegistry {
+ public:
+  AnswerCardContentsRegistry();
+  ~AnswerCardContentsRegistry();
+
+  static AnswerCardContentsRegistry* Get();
+
+  // Register content with a View.
+  base::UnguessableToken Register(views::View* contents_view);
+
+  // Unregister and release the associated resources.
+  void Unregister(const base::UnguessableToken& token);
+
+  // Get the view for the given token. Return nullptr for unknown token.
+  views::View* GetView(const base::UnguessableToken& token);
+
+ private:
+  std::map<base::UnguessableToken, views::View*> contents_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(AnswerCardContentsRegistry);
+};
+
+}  // namespace app_list
+
+#endif  // UI_APP_LIST_ANSWER_CARD_CONTENTS_REGISTRY_H_

ui/app_list/views/search_result_answer_card_view.cc

@@ -10,6 +10,7 @@
 
 #include "ui/accessibility/ax_node.h"
 #include "ui/accessibility/ax_node_data.h"
+#include "ui/app_list/answer_card_contents_registry.h"
 #include "ui/app_list/app_list_constants.h"
 #include "ui/app_list/app_list_metrics.h"
 #include "ui/app_list/app_list_view_delegate.h"
@@ -45,7 +46,10 @@ class SearchResultAnswerCardView::SearchAnswerContainerView
   bool SetSearchResult(SearchResult* search_result) {
     views::View* const old_result_view = child_count() ? child_at(0) : nullptr;
     views::View* const new_result_view =
-        search_result ? search_result->view() : nullptr;
+        search_result && AnswerCardContentsRegistry::Get()
+            ? AnswerCardContentsRegistry::Get()->GetView(
+                  search_result->answer_card_contents_token())
+            : nullptr;
 
     if (old_result_view != new_result_view) {
       if (old_result_view != nullptr)

ui/app_list/views/search_result_answer_card_view_unittest.cc

@@ -10,7 +10,9 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/unguessable_token.h"
 #include "ui/accessibility/ax_node_data.h"
+#include "ui/app_list/answer_card_contents_registry.h"
 #include "ui/app_list/app_list_constants.h"
 #include "ui/app_list/test/app_list_test_view_delegate.h"
 #include "ui/app_list/test/test_search_result.h"
@@ -43,6 +45,7 @@ class SearchResultAnswerCardViewTest : public views::ViewsTestBase {
 
     result_view_ = std::make_unique<views::View>();
     result_view_->set_owned_by_client();
+    token_ = contents_registry_.Register(result_view_.get());
 
     SetUpSearchResult();
   }
@@ -54,7 +57,7 @@ class SearchResultAnswerCardViewTest : public views::ViewsTestBase {
         std::make_unique<TestSearchResult>();
     result->set_display_type(SearchResult::DISPLAY_CARD);
     result->set_title(base::UTF8ToUTF16(kResultTitle));
-    result->set_view(result_view_.get());
+    result->set_answer_card_contents_token(token_);
     result->set_relevance(kRelevance);
     results->Add(std::move(result));
 
@@ -117,6 +120,9 @@ class SearchResultAnswerCardViewTest : public views::ViewsTestBase {
   // result_container_view_. Has set_owned_by_client() called.
   std::unique_ptr<views::View> result_view_;
 
+  AnswerCardContentsRegistry contents_registry_;
+  base::UnguessableToken token_;
+
   DISALLOW_COPY_AND_ASSIGN(SearchResultAnswerCardViewTest);
 };