New issue
Advanced search Search tips

Issue 750235 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task



Sign in to add a comment

Extract common base for HighlighterController and LaserPointerController

Project Member Reported by kaznacheev@chromium.org, Jul 28 2017

Issue description

HighlighterController is a modified copy of LaserPointerController. There is a lot of common behavior which should live in one place.

 
Status: Started (was: Assigned)
Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 4 2017

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

commit 35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb
Author: Vladislav Kaznacheev <kaznacheev@google.com>
Date: Fri Aug 04 22:40:26 2017

Introduce FastInkPointerController

A common base class for for LaserPointerController and
HighlighterController that incapsulates touch event
handling and manages the pointer view lifecycle.

Bug:  750235 
Change-Id: I82cade80bd192e08069da9a159c9ab592cdcc026
Reviewed-on: https://chromium-review.googlesource.com/600640
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492156}
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/BUILD.gn
[add] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/fast_ink/fast_ink_pointer_controller.cc
[add] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/fast_ink/fast_ink_pointer_controller.h
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/highlighter/highlighter_controller.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/highlighter/highlighter_controller.h
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/highlighter/highlighter_controller_test_api.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/highlighter/highlighter_result_view.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/highlighter/highlighter_result_view.h
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/highlighter/highlighter_view.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/highlighter/highlighter_view.h
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/laser/laser_pointer_controller.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/laser/laser_pointer_controller.h
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/laser/laser_pointer_controller_test_api.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/laser/laser_pointer_controller_test_api.h
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/laser/laser_pointer_controller_unittest.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/laser/laser_pointer_view.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/ash/laser/laser_pointer_view.h
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/chrome/browser/ui/ash/palette_delegate_chromeos.cc
[modify] https://crrev.com/35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb/chrome/browser/ui/ash/palette_delegate_chromeos.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 7 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49243d53ad59e0590f10cf606606fd043ba129e9

commit 49243d53ad59e0590f10cf606606fd043ba129e9
Author: Vladislav Kaznacheev <kaznacheev@google.com>
Date: Mon Aug 07 16:31:27 2017

Introduce FastInkPointerController

A common base class for for LaserPointerController and
HighlighterController that incapsulates touch event
handling and manages the pointer view lifecycle.

TBR=kaznacheev@google.com

(cherry picked from commit 35e2cdbdf0f1dbb7a7dc4d133ad0aac19c2fe6bb)

Bug:  750235 
Change-Id: I82cade80bd192e08069da9a159c9ab592cdcc026
Reviewed-on: https://chromium-review.googlesource.com/600640
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#492156}
Reviewed-on: https://chromium-review.googlesource.com/603968
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#348}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/BUILD.gn
[add] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/fast_ink/fast_ink_pointer_controller.cc
[add] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/fast_ink/fast_ink_pointer_controller.h
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/highlighter/highlighter_controller.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/highlighter/highlighter_controller.h
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/highlighter/highlighter_controller_test_api.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/highlighter/highlighter_result_view.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/highlighter/highlighter_result_view.h
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/highlighter/highlighter_view.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/highlighter/highlighter_view.h
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/laser/laser_pointer_controller.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/laser/laser_pointer_controller.h
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/laser/laser_pointer_controller_test_api.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/laser/laser_pointer_controller_test_api.h
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/laser/laser_pointer_controller_unittest.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/laser/laser_pointer_view.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/ash/laser/laser_pointer_view.h
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/chrome/browser/ui/ash/palette_delegate_chromeos.cc
[modify] https://crrev.com/49243d53ad59e0590f10cf606606fd043ba129e9/chrome/browser/ui/ash/palette_delegate_chromeos.h

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 8 2017

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 8 2017

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

commit 64d5218e2d2d590d9072f8ec69b2b18bfcb81178
Author: Vladislav Kaznacheev <kaznacheev@chromium.org>
Date: Tue Aug 08 23:06:36 2017

Revert "Add a test for highlighter/palette interaction"

This reverts commit e783956ccee92e62092e0f2dac984e219e241b84.

Reason for revert: Uninitialized pointer in TestPaletteDelegate
causing MetalayerToolTest to crash.

Original change's description:
> Add a test for highlighter/palette interaction
> 
> Provide additional test coverage for the interaction
> between HighlighterController and the stylus palette.
> 
> Bug:  750235 
> Change-Id: I067b34f6b9159b87a2aad4e5b66e245dc29d3a44
> Reviewed-on: https://chromium-review.googlesource.com/604108
> Reviewed-by: James Cook <jamescook@chromium.org>
> Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#492761}

TBR=jamescook@chromium.org,kaznacheev@chromium.org

Change-Id: I0af1b9977a3b14e316eaa2385d6f5454dd0753b5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  750235 
Reviewed-on: https://chromium-review.googlesource.com/607390
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492765}
[modify] https://crrev.com/64d5218e2d2d590d9072f8ec69b2b18bfcb81178/ash/highlighter/highlighter_controller_unittest.cc
[modify] https://crrev.com/64d5218e2d2d590d9072f8ec69b2b18bfcb81178/ash/system/palette/palette_tray_unittest.cc
[modify] https://crrev.com/64d5218e2d2d590d9072f8ec69b2b18bfcb81178/ash/system/palette/test_palette_delegate.cc
[modify] https://crrev.com/64d5218e2d2d590d9072f8ec69b2b18bfcb81178/ash/system/palette/test_palette_delegate.h
[modify] https://crrev.com/64d5218e2d2d590d9072f8ec69b2b18bfcb81178/ash/system/palette/tools/metalayer_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 9 2017

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

commit a2429dc8522667d64e8ff8b943ce02315a2169d0
Author: Vladislav Kaznacheev <kaznacheev@google.com>
Date: Wed Aug 09 05:20:31 2017

Reland: Add a test for highlighter/palette interaction

(Original CL reverted due to an uninitialized pointer)

Provide additional test coverage for the interaction
between HighlighterController and the stylus palette.

TBR=jamescook@chromium.org

Bug:  750235 
Change-Id: I69eee67062b515cc68557b19faf12ad93fd65600
Reviewed-on: https://chromium-review.googlesource.com/607467
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492833}
[modify] https://crrev.com/a2429dc8522667d64e8ff8b943ce02315a2169d0/ash/highlighter/highlighter_controller_unittest.cc
[modify] https://crrev.com/a2429dc8522667d64e8ff8b943ce02315a2169d0/ash/system/palette/palette_tray_unittest.cc
[modify] https://crrev.com/a2429dc8522667d64e8ff8b943ce02315a2169d0/ash/system/palette/test_palette_delegate.cc
[modify] https://crrev.com/a2429dc8522667d64e8ff8b943ce02315a2169d0/ash/system/palette/test_palette_delegate.h
[modify] https://crrev.com/a2429dc8522667d64e8ff8b943ce02315a2169d0/ash/system/palette/tools/metalayer_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 9 2017

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

commit 32cb6d39d884341a6e0da05757d89b639007a3d1
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Wed Aug 09 08:43:37 2017

Revert "Reland: Add a test for highlighter/palette interaction"

This reverts commit a2429dc8522667d64e8ff8b943ce02315a2169d0.

Reason for revert: This patch seems causing failure on MSan/LSan.
See also  https://crbug.com/753720  for detail.

Original change's description:
> Reland: Add a test for highlighter/palette interaction
> 
> (Original CL reverted due to an uninitialized pointer)
> 
> Provide additional test coverage for the interaction
> between HighlighterController and the stylus palette.
> 
> TBR=jamescook@chromium.org
> 
> Bug:  750235 
> Change-Id: I69eee67062b515cc68557b19faf12ad93fd65600
> Reviewed-on: https://chromium-review.googlesource.com/607467
> Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#492833}

TBR=jamescook@chromium.org,kaznacheev@chromium.org

Change-Id: Id70078cf9a48f73c5b8eb34a46f066cb2651de45
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  750235 
Reviewed-on: https://chromium-review.googlesource.com/606749
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492910}
[modify] https://crrev.com/32cb6d39d884341a6e0da05757d89b639007a3d1/ash/highlighter/highlighter_controller_unittest.cc
[modify] https://crrev.com/32cb6d39d884341a6e0da05757d89b639007a3d1/ash/system/palette/palette_tray_unittest.cc
[modify] https://crrev.com/32cb6d39d884341a6e0da05757d89b639007a3d1/ash/system/palette/test_palette_delegate.cc
[modify] https://crrev.com/32cb6d39d884341a6e0da05757d89b639007a3d1/ash/system/palette/test_palette_delegate.h
[modify] https://crrev.com/32cb6d39d884341a6e0da05757d89b639007a3d1/ash/system/palette/tools/metalayer_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 9 2017

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

commit 0e59c296609bb50adac568eaaefcaf1933c9ecb2
Author: Vladislav Kaznacheev <kaznacheev@google.com>
Date: Wed Aug 09 20:40:03 2017

Reland: Add a test for highlighter/palette interaction

(Fixed ASAN failures caused by using the highlighter
controller instance from the shell. Reverted to
using a test instance).

Provide additional test coverage for the interaction
between HighlighterController and the stylus palette.

Bug:  750235 
Change-Id: I9004477f3bede4a4d42d12ec48c9bd01ab3482c3
Reviewed-on: https://chromium-review.googlesource.com/609142
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493130}
[modify] https://crrev.com/0e59c296609bb50adac568eaaefcaf1933c9ecb2/ash/highlighter/highlighter_controller_unittest.cc
[modify] https://crrev.com/0e59c296609bb50adac568eaaefcaf1933c9ecb2/ash/system/palette/palette_tray_unittest.cc
[modify] https://crrev.com/0e59c296609bb50adac568eaaefcaf1933c9ecb2/ash/system/palette/test_palette_delegate.cc
[modify] https://crrev.com/0e59c296609bb50adac568eaaefcaf1933c9ecb2/ash/system/palette/test_palette_delegate.h
[modify] https://crrev.com/0e59c296609bb50adac568eaaefcaf1933c9ecb2/ash/system/palette/tools/metalayer_unittest.cc

Comment 11 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment