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

Issue 833259 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 829383



Sign in to add a comment

Inline Reply on ARC notifications doesn't work

Project Member Reported by yoshiki@chromium.org, Apr 16 2018

Issue description

It breaks for some reason. I'm assuming it's a regression on Chrome-side.

Good: 10553.0.0(67.0.3383.0)
Bad: 10569.1.0(67.0.3383.0)

We may need bisect. But notification is broken between the good and the bad builds, so that we may need to apply the patch manually during bisect.
 
Cc: yoshiki@chromium.org
Owner: yhanada@chromium.org
As I did bisect, Hanada-san's change is a cause of the regression:
https://chromium-review.googlesource.com/c/chromium/src/+/1001086

Hanada-san, could you take a look?
Labels: M-67
I guess the CL is in M-67.
Blockedon: 829383
Project Member

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

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

commit 6f66fc4e3d7da3cfa2afed5f8fd82f70fa5c28b6
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Wed Apr 18 15:49:13 2018

Make IsArcWindow() return true for notification surfaces.

Checking a window name is OK for now because
NotificationSurfaces are not used outside ARC++.
We will set an application id for a notification surface and use it in a
follow up CL.

Bug:  829383 ,  833259 
Test: Manual. Inline reply on ARC notifications works.
Change-Id: Icf3d43df527988f92ca88f7b89b673fdd02b00bc
Reviewed-on: https://chromium-review.googlesource.com/1015440
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551694}
[modify] https://crrev.com/6f66fc4e3d7da3cfa2afed5f8fd82f70fa5c28b6/components/arc/ime/arc_ime_service.cc

Labels: Merge-Request-67
requesting to merge the CL in comment 4 to M-67
Biggish change for a merge request.  Has this been tested on ToT for a number of boards.  Thanks.
Sorry, #6 was a question re: testing.
I verified it on soraka, veyron_minnie, lux, caroline and dru.
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 25 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 25 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/395c5df65759b1c8790b16dbd3b1f28fd47395ea

commit 395c5df65759b1c8790b16dbd3b1f28fd47395ea
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Wed Apr 25 21:38:46 2018

Make IsArcWindow() return true for notification surfaces.

Checking a window name is OK for now because
NotificationSurfaces are not used outside ARC++.
We will set an application id for a notification surface and use it in a
follow up CL.

Bug:  829383 ,  833259 
Test: Manual. Inline reply on ARC notifications works.
Change-Id: Icf3d43df527988f92ca88f7b89b673fdd02b00bc
Reviewed-on: https://chromium-review.googlesource.com/1015440
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551694}(cherry picked from commit 6f66fc4e3d7da3cfa2afed5f8fd82f70fa5c28b6)
Reviewed-on: https://chromium-review.googlesource.com/1028890
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#310}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/395c5df65759b1c8790b16dbd3b1f28fd47395ea/components/arc/ime/arc_ime_service.cc

Status: Fixed (was: Started)

Sign in to add a comment