Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2889183004:
Exclude Android system windows from a voice interaction snapshsot. (Closed)
|
Created:
3 years, 7 months ago by Vladislav Kaznacheev Modified:
3 years, 7 months ago CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, oshima+watch_chromium.org, yusukes+watch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, victorhsieh+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionExclude Android system windows from a voice interaction snapshsot.
For the best UX the metalayer has to be excluded from the snapshot.
It is currently impossible to identify the metalayer among others
system windows. Other layers under kShellWindowId_SystemModalContainer
are not relevant for this kind of snapshot, so it is safe to exclude
all of them.
BUG=b:37640737
TEST=Invoke metalayer, check that the screenshot does not contain
stylus strokes.
Review-Url: https://codereview.chromium.org/2889183004
Cr-Commit-Position: refs/heads/master@{#473250}
Committed: https://chromium.googlesource.com/chromium/src/+/e2c22cf45e4b3017b377a6967b6314a3adcaedca
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed comments #Messages
Total messages: 16 (8 generated)
kaznacheev@chromium.org changed reviewers: + lhchavez@lhchavez.com, xiaohuic@google.com
Please review.
xiaohuic@chromium.org changed reviewers: + xiaohuic@chromium.org
Commit message has extra BUG= field. https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... File chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc (right): https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:70: root_window, ash::kShellWindowId_SystemModalContainer); Why metalayer view is in this container?
Description was changed from ========== Exclude Android system windows from a voice interaction snapshsot. For the best UX the metalayer has to be excluded from the snapshot. It is currently impossible to identify the metalayer among others system windows. Other layers under kShellWindowId_SystemModalContainer are not relevant for this kind of snapshot, so it is safe to exclude all of them. BUG= BUG: b:37640737 TEST: Invoke metalayer, check that the screenshot does not contain stylus strokes. ========== to ========== Exclude Android system windows from a voice interaction snapshsot. For the best UX the metalayer has to be excluded from the snapshot. It is currently impossible to identify the metalayer among others system windows. Other layers under kShellWindowId_SystemModalContainer are not relevant for this kind of snapshot, so it is safe to exclude all of them. BUG=b:37640737 TEST=Invoke metalayer, check that the screenshot does not contain stylus strokes. ==========
kaznacheev@google.com changed reviewers: + kaznacheev@google.com
https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... File chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc (right): https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:70: root_window, ash::kShellWindowId_SystemModalContainer); On 2017/05/19 00:10:42, xc wrote: > Why metalayer view is in this container? This is where Chrome puts all Android system windows (that is, windows with null application token).
lhchavez@chromium.org changed reviewers: + lhchavez@chromium.org - lhchavez@lhchavez.com
Adding the correct lhchavez. https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... File chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc (right): https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:71: if (modal_container != nullptr) { nit: elide braces. Same in L91.
PTAL https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... File chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc (right): https://codereview.chromium.org/2889183004/diff/1/chrome/browser/chromeos/arc... chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:71: if (modal_container != nullptr) { On 2017/05/19 15:03:21, Luis Héctor Chávez wrote: > nit: elide braces. Same in L91. Done.
lgtm
The CQ bit was checked by kaznacheev@google.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495214094196600,
"parent_rev": "59b25f6c12708adf7dbce3a4e08eb15ab845626b", "commit_rev":
"e2c22cf45e4b3017b377a6967b6314a3adcaedca"}
Message was sent while issue was closed.
Description was changed from ========== Exclude Android system windows from a voice interaction snapshsot. For the best UX the metalayer has to be excluded from the snapshot. It is currently impossible to identify the metalayer among others system windows. Other layers under kShellWindowId_SystemModalContainer are not relevant for this kind of snapshot, so it is safe to exclude all of them. BUG=b:37640737 TEST=Invoke metalayer, check that the screenshot does not contain stylus strokes. ========== to ========== Exclude Android system windows from a voice interaction snapshsot. For the best UX the metalayer has to be excluded from the snapshot. It is currently impossible to identify the metalayer among others system windows. Other layers under kShellWindowId_SystemModalContainer are not relevant for this kind of snapshot, so it is safe to exclude all of them. BUG=b:37640737 TEST=Invoke metalayer, check that the screenshot does not contain stylus strokes. Review-Url: https://codereview.chromium.org/2889183004 Cr-Commit-Position: refs/heads/master@{#473250} Committed: https://chromium.googlesource.com/chromium/src/+/e2c22cf45e4b3017b377a6967b63... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/e2c22cf45e4b3017b377a6967b63... |
