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

Issue 854787 link

Starred by 1 user
Status: Fixed
Owner:
xiy...@chromium.org
Closed: Aug 15
Cc:
█ vadimt@chromium.org
xiy...@chromium.org
█ hejq@chromium.org
dmblack@google.com
jamescook@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 678705


Show other hotlists

Hotlists containing this issue:
Launcher-Tech-Debt


Sign in to add a comment

AnswerCardWebContents / WebContentsManager needs to support mash

Project Member Reported by steve...@chromium.org, Jun 20 2018 
I couldn't find an existing bug for this, despite some notes and a TODO.

AnswerCardWebContents has a dependency on AppListView::ExcludeWindowFromEventHandling (which seems to exist only for this dependency).

ExcludeWindowFromEventHandling has the comment:
"Prevents handling input events for the |window| in context of handling in app list."

The call is from SearchAnswerWebView in answer_card_web_contents.cc, which is only used when AnswerCardContentsRegistry::Get() is non null, which seems to be non mash only.

ManagedWebContents in WebContetsManager (in c/b/ui/ash/assistant) has a comment: 'TODO(dmblack): Handle Mash case' when AnswerCardContentsRegistry::Get() returns null.

Comment 1 by steve...@chromium.org, Jun 20 2018

Blocking: 678705

Comment 2 by xiy...@chromium.org, Jun 20 2018

Owner: xiy...@chromium.org
Status: Assigned (was: Untriaged)
Most of it should work with  issue 812434 .

I'll take this one and get rid of the include.

Comment 3 by xiy...@chromium.org, Jun 20 2018 

More specifically, I will move AppListView::ExcludeWindowFromEventHandling call from the browser side to ash side.

WebContentsManager work is still TBD.

Comment 4 by dmblack@google.com, Jun 20 2018 

I can take a look at mirroring what was done for AnswerCardWebContents in WebContentsManager.

Comment 5 by dxie@google.com, Aug 13

Labels: Proj-Mustash
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 14 

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

commit 8dc4077b32db61641d793f4d6ce363b343d72858
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue Aug 14 23:26:25 2018

mash: Clean up AnswerCardWebContents deps

Move AppListView::ExcludeWindowFromEventHandling from browser process
to app list code in ash process.

Bug:  854787 
Change-Id: I7509fdd8c5cc4ea0db211bdc4c7fe8715fb10660
Reviewed-on: https://chromium-review.googlesource.com/1170036
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583075}
[modify] https://crrev.com/8dc4077b32db61641d793f4d6ce363b343d72858/ash/app_list/views/app_list_view_unittest.cc
[modify] https://crrev.com/8dc4077b32db61641d793f4d6ce363b343d72858/ash/app_list/views/search_result_answer_card_view.cc
[modify] https://crrev.com/8dc4077b32db61641d793f4d6ce363b343d72858/ash/app_list/views/search_result_answer_card_view_unittest.cc
[modify] https://crrev.com/8dc4077b32db61641d793f4d6ce363b343d72858/ash/public/cpp/app_list/answer_card_contents_registry.cc
[modify] https://crrev.com/8dc4077b32db61641d793f4d6ce363b343d72858/ash/public/cpp/app_list/answer_card_contents_registry.h
[modify] https://crrev.com/8dc4077b32db61641d793f4d6ce363b343d72858/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.cc
[modify] https://crrev.com/8dc4077b32db61641d793f4d6ce363b343d72858/chrome/browser/ui/ash/assistant/web_contents_manager.cc

Comment 7 by xiy...@chromium.org, Aug 15

Status: Fixed (was: Assigned)
►

Sign in to add a comment