New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 812434 link

Starred by 0 users
Status: Fixed
Owner:
xiy...@chromium.org
Closed: Aug 23
Cc:
mustash-bugs@google.com
█ hejq@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task

Blocked on:
issue 876513
Blocking:
issue 733662


Show other hotlists

Hotlists containing this issue:
Launcher-Tech-Debt


Sign in to add a comment

Launcher answer card in mash

Project Member Reported by xiy...@chromium.org, Feb 15 2018 
Tracks the work to support answser card in mash.

Launcher answer card is loaded in content::WebContents and passed from browser to ui/app_list code via a views::WebView*. This is not going to work in mash. 

We need something that can go through mojo for ash process to embed in its UI. Current thinking is to pass a WindowTreeClient for the WebContents.

Comment 1 by xiy...@chromium.org, Feb 15 2018

Blocking: 733662

Comment 2 by lafo...@chromium.org, Feb 26 2018

Components: -Internals>MUS Internals>Services>WindowService
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 15 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d94f7320abebb53842d2151e9b8fd893390abfb2

commit d94f7320abebb53842d2151e9b8fd893390abfb2
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Thu Mar 15 14:40:56 2018

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/957742
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543368}
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ash/app_list/app_list_controller_impl.cc
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ash/app_list/app_list_controller_impl.h
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ash/app_list/model/search/search_model.cc
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ash/app_list/model/search/search_result.h
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/chrome/browser/ui/app_list/search/answer_card/answer_card_contents.h
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/chrome/browser/ui/app_list/search/answer_card/answer_card_result.cc
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/chrome/browser/ui/app_list/search/answer_card/answer_card_result_unittest.cc
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/chrome/browser/ui/app_list/search/answer_card/answer_card_search_provider_unittest.cc
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.cc
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.h
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ui/app_list/BUILD.gn
[add] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ui/app_list/answer_card_contents_registry.cc
[add] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ui/app_list/answer_card_contents_registry.h
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ui/app_list/views/search_result_answer_card_view.cc
[modify] https://crrev.com/d94f7320abebb53842d2151e9b8fd893390abfb2/ui/app_list/views/search_result_answer_card_view_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 4 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bfdbf9f355a14348c06bba3000af132f238dd38

commit 0bfdbf9f355a14348c06bba3000af132f238dd38
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Wed Apr 04 21:47:10 2018

Helper to use window tree embed API

- RemoteViewHost derived from NativeViewHost on embedder side to embed
  a window and host the embedding window in view hierarchy;
- RemoteViewProvider on the embedded side to prepare an aura::Window
  to be embedded;

Bug:  812434 
Test: views_mus_unittests --gtest_filter=RemoteView*
Change-Id: I1c999bf07d16330cf8b97df5e7f55b5bbcaa377a
Reviewed-on: https://chromium-review.googlesource.com/989014
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548213}
[modify] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/aura/test/mus/test_window_tree.cc
[modify] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/aura/test/mus/test_window_tree.h
[modify] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/BUILD.gn
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/BUILD.gn
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/README.md
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_host.cc
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_host.h
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_host_unittest.cc
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_provider.cc
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_provider.h
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_provider_test_api.cc
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_provider_test_api.h
[add] https://crrev.com/0bfdbf9f355a14348c06bba3000af132f238dd38/ui/views/mus/remote_view/remote_view_provider_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 5 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9d7693bb65b211c9f175c393f59c50260e63800

commit b9d7693bb65b211c9f175c393f59c50260e63800
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Thu Apr 05 19:45:51 2018

RemoteViewHost embeds after added to a widget

The embedding aura::Window must be attached to a root window so that
window server could figure out the correct display and pass it to the
other side. Deferring the EmbedUsingToken call until RemoteViewHost
is added to a widget.

Bug:  812434 
Change-Id: Ie65d12d8b883b1a74426c33c3f106bbb637c0f69
Reviewed-on: https://chromium-review.googlesource.com/998498
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548529}
[modify] https://crrev.com/b9d7693bb65b211c9f175c393f59c50260e63800/ui/views/mus/remote_view/remote_view_host.cc
[modify] https://crrev.com/b9d7693bb65b211c9f175c393f59c50260e63800/ui/views/mus/remote_view/remote_view_host.h
[modify] https://crrev.com/b9d7693bb65b211c9f175c393f59c50260e63800/ui/views/mus/remote_view/remote_view_host_unittest.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 6 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff

commit 6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri Apr 06 17:23:55 2018

app_list: Answer card in mash

- AnswerCardWebContents uses RemoteViewProvider to prepare answer card
  contents for embedding;
- SearchResultAnswerCardView uses RemoteViewHost to embed the
  contents from AnserCardWebContents;

Bug:  812434 
Change-Id: I2c9785c96987367a72278c66205f20e579bf91ca
Reviewed-on: https://chromium-review.googlesource.com/992361
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548835}
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/chrome/browser/ui/app_list/search/answer_card/answer_card_contents.h
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/chrome/browser/ui/app_list/search/answer_card/answer_card_search_provider.cc
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/chrome/browser/ui/app_list/search/answer_card/answer_card_search_provider.h
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/chrome/browser/ui/app_list/search/answer_card/answer_card_search_provider_unittest.cc
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.cc
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.h
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/ui/app_list/BUILD.gn
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/ui/app_list/views/app_list_view_unittest.cc
[modify] https://crrev.com/6f339856ad10cf4cb20ae1f9be6aa9b5bb7560ff/ui/app_list/views/search_result_answer_card_view.cc
Project Member

Comment 7 by bugdroid1@chromium.org, May 16 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/557d8e0e335ab7234fa1671bd70704659b675efa

commit 557d8e0e335ab7234fa1671bd70704659b675efa
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Wed May 16 17:39:22 2018

mash: Enable answer card

- Add kAnswerCard result type for answer card result;
- Pass answer card preferred size via mojo;
- SearchResultAnswerCardView creates RemoteViewHost only when the
  embedding token changes because the token could be used only once;
- SearchResultWebContents re-creates RemoteViewProvider to ensure
  a valid embedding token in case existing token is invalidated
  when swapping cards but the browser is not aware of that when
  creating a new card;

Bug:  812434 
Change-Id: I2a8b61974c46b779d8a61a571e95d8003b2bc123
Reviewed-on: https://chromium-review.googlesource.com/1060280
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559167}
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/ash/app_list/model/search/search_result.h
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/ash/public/cpp/app_list/app_list_struct_traits.h
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/ash/public/cpp/app_list/app_list_types.h
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/ash/public/interfaces/app_list.mojom
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/answer_card/answer_card_contents.h
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/answer_card/answer_card_result.cc
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/answer_card/answer_card_result_unittest.cc
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/answer_card/answer_card_search_provider_unittest.cc
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.cc
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.h
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/chrome_search_result.cc
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/chrome_search_result.h
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/chrome/browser/ui/app_list/search/search_controller_factory.cc
[modify] https://crrev.com/557d8e0e335ab7234fa1671bd70704659b675efa/ui/app_list/views/search_result_answer_card_view.cc

Comment 8 by xiy...@chromium.org, Jun 4 2018

Status: Fixed (was: Assigned)
Claiming this fixed.

The known issue is that the WebContentsAura gets embedded would lose its children when gets unembedded because ash process is special and sees all the embedded window hierarchy. As a result, each WebContents (card) is visible only once.

The problem should be gone how ash manages window trees is changed.

Comment 10 by xiy...@chromium.org, Jun 4 2018 

Re #9: We should figure out a way to plumb this. But IMO that is a different bug since this one is to support mash answer card and the code is for classic ash.

Comment 11 by xiy...@chromium.org, Aug 9

Status: Started (was: Fixed)
Re-open to make it work with ws2

Comment 12 by xiy...@chromium.org, Aug 21

Blockedon: 876513
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c3a585b0081f8224cc351f1af70b4d8476d5fbf

commit 0c3a585b0081f8224cc351f1af70b4d8476d5fbf
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Thu Aug 23 21:15:11 2018

app_list: Use AppListEventTargeter for mash answer card

- kEmbedFlagEmbedderInterceptsEvents is not yet supported for
  ServerRemoveViewHost embedding. Change it to a DCHECK.
- Use AppListEventTargeter for mash card like in classic ash.

Bug:  812434 
Change-Id: Ie2a952b6e60b66c4f87855fc85313505d0586a2b
Reviewed-on: https://chromium-review.googlesource.com/1187185
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585616}
[modify] https://crrev.com/0c3a585b0081f8224cc351f1af70b4d8476d5fbf/ash/app_list/views/search_result_answer_card_view.cc
[modify] https://crrev.com/0c3a585b0081f8224cc351f1af70b4d8476d5fbf/services/ui/ws2/embedding.cc
[modify] https://crrev.com/0c3a585b0081f8224cc351f1af70b4d8476d5fbf/services/ui/ws2/remote_view_host/server_remote_view_host.cc
[modify] https://crrev.com/0c3a585b0081f8224cc351f1af70b4d8476d5fbf/services/ui/ws2/remote_view_host/server_remote_view_host.h
[modify] https://crrev.com/0c3a585b0081f8224cc351f1af70b4d8476d5fbf/services/ui/ws2/window_service.cc

Comment 14 by xiy...@chromium.org, Aug 23

Status: Fixed (was: Started)
►

Sign in to add a comment