Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1828133004:
Refine ash shelf tooltip closing on non-mash ChromeOS. (Closed)
|
Created:
4 years, 9 months ago by msw Modified:
4 years, 8 months ago Reviewers:
sky CC:
chromium-reviews, sadrul, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, kalyank, darin (slow to review), ben+mojo_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRefine ash shelf tooltip closing on non-mash ChromeOS.
Fixes a regression from: https://codereview.chromium.org/1816753002
Also reverse expected behavior for touching tooltips.
(closes on stable, tests expected them to stay open...)
Inline bubble function definitions.
Use WindowObserver to remove the handler on window destruction.
TODO: Encapsulate more ShelfLayoutManager code.
BUG=595853
TEST=ChromeOS tooltips stay open on hover; close for external touches.
R=sky@chromium.org
Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24
Cr-Commit-Position: refs/heads/master@{#383404}
Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38
Cr-Commit-Position: refs/heads/master@{#383579}
Committed: https://crrev.com/5b1ca37ffa7c373b9b726f786f410bc80348a246
Cr-Commit-Position: refs/heads/master@{#383801}
Patch Set 1 #Patch Set 2 : Refine; update tests. #Patch Set 3 : Notify WillDeleteShelf on ShelfLayoutManager::PrepareForShutdown. #Patch Set 4 : Don't clear the observer list from ShelfLayoutManager. #Patch Set 5 : Remove observer in ShelfWindowTargeter::ShelfWindowTargeter. #Patch Set 6 : Avoid WillDeleteShelf in PrepareForShutdown; use aura::WindowObserver instead. #
Messages
Total messages: 44 (25 generated)
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on sTable, tests expected them to stay open...) BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org ==========
The CQ bit was checked by msw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1828133004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1828133004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
The CQ bit was checked by msw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1828133004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1828133004/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on sTable, tests expected them to stay open...) BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org ==========
Hey Scott, please take a look; thanks!
LGTM
The CQ bit was checked by msw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org Link to the patchset: https://codereview.chromium.org/1828133004/#ps60001 (title: "Don't clear the observer list from ShelfLayoutManager.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1828133004/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1828133004/60001
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404}
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/1836753002/ by msw@chromium.org. The reason for reverting is: Memory failures, eg. https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20ChromeOS%20....
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} ==========
The CQ bit was checked by msw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1828133004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1828133004/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by msw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org Link to the patchset: https://codereview.chromium.org/1828133004/#ps80001 (title: "Remove observer in ShelfWindowTargeter::ShelfWindowTargeter.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1828133004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1828133004/80001
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579}
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/1834203003/ by msw@chromium.org. The reason for reverting is: ash_unittests ShelfLayoutManagerTest.ShutdownHandlesWindowActivation failure on win: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2....
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ==========
Patchset #6 (id:100001) has been deleted
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Call WillDeleteShelf earlier to fix a test crash... TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Use WindowObserver to remove the handler on window destruction. TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ==========
Patchset #6 (id:120001) has been deleted
The CQ bit was checked by msw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org Link to the patchset: https://codereview.chromium.org/1828133004/#ps140001 (title: "Avoid WillDeleteShelf in PrepareForShutdown; use aura::WindowObserver instead.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1828133004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1828133004/140001
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Use WindowObserver to remove the handler on window destruction. TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Use WindowObserver to remove the handler on window destruction. TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ==========
Message was sent while issue was closed.
Committed patchset #6 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Use WindowObserver to remove the handler on window destruction. TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} ========== to ========== Refine ash shelf tooltip closing on non-mash ChromeOS. Fixes a regression from: https://codereview.chromium.org/1816753002 Also reverse expected behavior for touching tooltips. (closes on stable, tests expected them to stay open...) Inline bubble function definitions. Use WindowObserver to remove the handler on window destruction. TODO: Encapsulate more ShelfLayoutManager code. BUG=595853 TEST=ChromeOS tooltips stay open on hover; close for external touches. R=sky@chromium.org Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24 Cr-Commit-Position: refs/heads/master@{#383404} Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38 Cr-Commit-Position: refs/heads/master@{#383579} Committed: https://crrev.com/5b1ca37ffa7c373b9b726f786f410bc80348a246 Cr-Commit-Position: refs/heads/master@{#383801} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/5b1ca37ffa7c373b9b726f786f410bc80348a246 Cr-Commit-Position: refs/heads/master@{#383801} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
