Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(538)

Issue 2234203002: Auto open and close the palette on an eject event. (Closed)

Created:
4 years, 4 months ago by jdufault
Modified:
4 years, 3 months ago
Reviewers:
stevenjb, oshima
CC:
chromium-reviews, kalyank, sadrul, oshima+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@tool-note
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Auto open and close the palette on an eject event. BUG=616112 TBR=oshima@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/d57fbda9be63adc83e769c52759d9a7e55b65225 Cr-Commit-Position: refs/heads/master@{#414592}

Patch Set 1 : Initial upload #

Total comments: 6

Patch Set 2 : Rename method in parent CL #

Patch Set 3 : Call bubble_.reset() directly #

Patch Set 4 : Rebase #

Patch Set 5 : Fix compile #

Patch Set 6 : Rebase onto master #

Patch Set 7 : Rebase on master #

Total comments: 8

Patch Set 8 : Address comments #

Patch Set 9 : Fix mash #

Patch Set 10 : Make PaletteDelegate pure virtual once more #

Total comments: 2

Patch Set 11 : Use weak ptr #

Unified diffs Side-by-side diffs Delta from patch set Stats (+102 lines, -15 lines) Patch
M ash/common/palette_delegate.h View 1 2 3 4 5 6 7 8 9 3 chunks +12 lines, -0 lines 0 comments Download
M ash/common/system/chromeos/palette/palette_tray.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +7 lines, -0 lines 0 comments Download
M ash/common/system/chromeos/palette/palette_tray.cc View 1 2 3 4 5 6 7 8 9 10 6 chunks +20 lines, -2 lines 0 comments Download
M ash/common/test/test_palette_delegate.h View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -1 line 0 comments Download
M ash/common/test/test_palette_delegate.cc View 1 2 3 4 5 6 7 8 1 chunk +8 lines, -0 lines 0 comments Download
M ash/shell/shell_delegate_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/preferences.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/settings_private/prefs_util.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/options/chromeos/note_overlay.html View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/settings/device_page/note.html View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/ash/palette_delegate_chromeos.h View 1 2 3 4 5 6 7 8 2 chunks +11 lines, -1 line 0 comments Download
M chrome/browser/ui/ash/palette_delegate_chromeos.cc View 1 2 3 4 5 6 7 8 4 chunks +23 lines, -2 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -4 lines 0 comments Download

Messages

Total messages: 96 (62 generated)
jdufault
oshima@, PTAL at ash/* stevenjb@, PTAL at chrome/* Thanks!
4 years, 4 months ago (2016-08-12 20:51:17 UTC) #24
jdufault
stevenjb@, please also take a look at ash/*. Thanks!
4 years, 4 months ago (2016-08-12 22:21:37 UTC) #25
stevenjb
https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/eject_controller.cc File ash/common/system/chromeos/palette/eject_controller.cc (right): https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/eject_controller.cc#newcode22 ash/common/system/chromeos/palette/eject_controller.cc:22: bool ejected = ui::InputDeviceManager::GetInstance()->StylusEject(); I can't find the CL ...
4 years, 4 months ago (2016-08-15 20:18:25 UTC) #26
jdufault
https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/eject_controller.cc File ash/common/system/chromeos/palette/eject_controller.cc (right): https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/eject_controller.cc#newcode22 ash/common/system/chromeos/palette/eject_controller.cc:22: bool ejected = ui::InputDeviceManager::GetInstance()->StylusEject(); On 2016/08/15 20:18:25, stevenjb wrote: ...
4 years, 4 months ago (2016-08-15 21:55:04 UTC) #27
stevenjb
https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/palette_tray.cc File ash/common/system/chromeos/palette/palette_tray.cc (right): https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/palette_tray.cc#newcode273 ash/common/system/chromeos/palette/palette_tray.cc:273: void PaletteTray::HidePalette() { On 2016/08/15 21:55:04, jdufault wrote: > ...
4 years, 4 months ago (2016-08-15 22:41:29 UTC) #28
jdufault
https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/palette_tray.cc File ash/common/system/chromeos/palette/palette_tray.cc (right): https://codereview.chromium.org/2234203002/diff/80001/ash/common/system/chromeos/palette/palette_tray.cc#newcode273 ash/common/system/chromeos/palette/palette_tray.cc:273: void PaletteTray::HidePalette() { On 2016/08/15 22:41:29, stevenjb wrote: > ...
4 years, 4 months ago (2016-08-16 00:02:18 UTC) #29
stevenjb
lgtm
4 years, 4 months ago (2016-08-16 16:18:11 UTC) #30
jdufault
TBR oshima@ for trivial changes on ash/common/palette_delegate.h and ash/shell/shell_delegate_impl.cc.
4 years, 4 months ago (2016-08-23 18:14:10 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/140001
4 years, 4 months ago (2016-08-23 18:14:57 UTC) #35
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-generic_chromium_compile_only_ng/builds/188115) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
4 years, 4 months ago (2016-08-23 18:30:58 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/160001
4 years, 4 months ago (2016-08-23 22:11:42 UTC) #40
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/266254)
4 years, 4 months ago (2016-08-24 00:03:34 UTC) #42
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/160001
4 years, 4 months ago (2016-08-24 01:00:25 UTC) #44
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/244479)
4 years, 4 months ago (2016-08-24 01:08:43 UTC) #46
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/180001
4 years, 4 months ago (2016-08-24 17:56:34 UTC) #49
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/129024)
4 years, 4 months ago (2016-08-24 19:44:50 UTC) #51
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/180001
4 years, 4 months ago (2016-08-24 20:48:22 UTC) #53
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/129238) chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, ...
4 years, 4 months ago (2016-08-24 20:54:27 UTC) #55
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/200001
4 years, 4 months ago (2016-08-24 21:15:04 UTC) #58
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/267019)
4 years, 4 months ago (2016-08-24 23:16:25 UTC) #60
oshima
lgtm with nits https://codereview.chromium.org/2234203002/diff/200001/ash/common/system/chromeos/palette/eject_controller.h File ash/common/system/chromeos/palette/eject_controller.h (right): https://codereview.chromium.org/2234203002/diff/200001/ash/common/system/chromeos/palette/eject_controller.h#newcode21 ash/common/system/chromeos/palette/eject_controller.h:21: EjectController(const OnEjectCallback& on_eject); nit: explicit https://codereview.chromium.org/2234203002/diff/200001/ash/common/system/chromeos/palette/palette_tray.cc ...
4 years, 4 months ago (2016-08-24 23:38:50 UTC) #61
jdufault
https://codereview.chromium.org/2234203002/diff/200001/ash/common/system/chromeos/palette/eject_controller.h File ash/common/system/chromeos/palette/eject_controller.h (right): https://codereview.chromium.org/2234203002/diff/200001/ash/common/system/chromeos/palette/eject_controller.h#newcode21 ash/common/system/chromeos/palette/eject_controller.h:21: EjectController(const OnEjectCallback& on_eject); On 2016/08/24 23:38:50, oshima wrote: > ...
4 years, 4 months ago (2016-08-24 23:54:08 UTC) #62
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/220001
4 years, 4 months ago (2016-08-24 23:55:45 UTC) #65
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/267116)
4 years, 4 months ago (2016-08-25 00:45:35 UTC) #67
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/220001
4 years, 4 months ago (2016-08-25 01:54:52 UTC) #69
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/267228)
4 years, 3 months ago (2016-08-25 03:26:25 UTC) #71
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/220001
4 years, 3 months ago (2016-08-25 17:55:16 UTC) #73
jdufault
Sorry the run around. PTAL again. The previous implementation ended up crashing mash - when ...
4 years, 3 months ago (2016-08-25 19:26:15 UTC) #80
stevenjb
https://codereview.chromium.org/2234203002/diff/280001/ash/common/system/chromeos/palette/palette_tray.cc File ash/common/system/chromeos/palette/palette_tray.cc (right): https://codereview.chromium.org/2234203002/diff/280001/ash/common/system/chromeos/palette/palette_tray.cc#newcode141 ash/common/system/chromeos/palette/palette_tray.cc:141: base::Bind(&PaletteTray::OnStylusStateChanged, base::Unretained(this))); Need to pass this as a weak_ptr. ...
4 years, 3 months ago (2016-08-25 22:48:56 UTC) #87
jdufault
https://codereview.chromium.org/2234203002/diff/280001/ash/common/system/chromeos/palette/palette_tray.cc File ash/common/system/chromeos/palette/palette_tray.cc (right): https://codereview.chromium.org/2234203002/diff/280001/ash/common/system/chromeos/palette/palette_tray.cc#newcode141 ash/common/system/chromeos/palette/palette_tray.cc:141: base::Bind(&PaletteTray::OnStylusStateChanged, base::Unretained(this))); On 2016/08/25 22:48:56, stevenjb wrote: > Need ...
4 years, 3 months ago (2016-08-25 23:09:15 UTC) #88
stevenjb
lgtm
4 years, 3 months ago (2016-08-25 23:09:59 UTC) #89
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2234203002/300001
4 years, 3 months ago (2016-08-25 23:11:43 UTC) #92
commit-bot: I haz the power
Committed patchset #11 (id:300001)
4 years, 3 months ago (2016-08-26 00:18:36 UTC) #94
commit-bot: I haz the power
4 years, 3 months ago (2016-08-26 00:31:04 UTC) #96
Message was sent while issue was closed.
Patchset 11 (id:??) landed as
https://crrev.com/d57fbda9be63adc83e769c52759d9a7e55b65225
Cr-Commit-Position: refs/heads/master@{#414592}

Powered by Google App Engine
This is Rietveld 408576698