Issue metadata
Sign in to add a comment
|
FocusManagerTest.ViewFocusCallbacks is Flaky |
||||||||||||||||||||
Issue descriptionFindit has detected flake occurrences for the test FocusManagerTest.ViewFocusCallbacks Culprit (92.6% confidence): https://chromium-review.googlesource.com/q/Iee8efd8895284acca4603ff22bde19adf16d8a7f Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVypwELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJxY2hyb21pdW0uY2hyb21pdW1vcy9saW51eC1jaHJvbWVvcy1yZWwvMTE2MDMvdmlld3NfbXVzX3VuaXR0ZXN0cy9SbTlqZFhOTllXNWhaMlZ5VkdWemRDNVdhV1YzUm05amRYTkRZV3hzWW1GamEzTT0MCxITTWFzdGVyRmxha2VBbmFseXNpcxgBDA Please revert the culprit, or disable the test and find the appropriate owner. https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Analyzer%20-%20Wrong%20result%20for%20FocusManagerTest.ViewFocusCallbacks&comment=Link%20to%20Analysis%3A%20https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVypwELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJxY2hyb21pdW0uY2hyb21pdW1vcy9saW51eC1jaHJvbWVvcy1yZWwvMTE2MDMvdmlld3NfbXVzX3VuaXR0ZXN0cy9SbTlqZFhOTllXNWhaMlZ5VkdWemRDNVdhV1YzUm05amRYTkRZV3hzWW1GamEzTT0MCxITTWFzdGVyRmxha2VBbmFseXNpcxgBDA
,
Aug 7
Revert triggered.
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd86dba05535e8b4fb9b2eafa88cd96be93f18f4 commit cd86dba05535e8b4fb9b2eafa88cd96be93f18f4 Author: Rakina Zata Amni <rakina@chromium.org> Date: Tue Aug 07 07:16:00 2018 Disable flaky FocusManagerTest.ViewFocusCallbacks TBR=msw@chromium.org Bug: 871652 Change-Id: Ic5ecabf54b1915a3c29cdf65d4bcc4e400300840 Reviewed-on: https://chromium-review.googlesource.com/1164805 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/heads/master@{#581159} [modify] https://crrev.com/cd86dba05535e8b4fb9b2eafa88cd96be93f18f4/ui/views/focus/focus_manager_unittest.cc
,
Aug 7
Findit identified the culprit r581113 with confidence 78.7% in the config "chromium.chromiumos / linux-chromeos-dbg" based on the flakiness trend: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVypgELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJwY2hyb21pdW0uY2hyb21pdW1vcy9saW51eC1jaHJvbWVvcy1kYmcvNzE4MS92aWV3c19tdXNfdW5pdHRlc3RzL1JtOWpkWE5OWVc1aFoyVnlWR1Z6ZEM1V2FXVjNSbTlqZFhORFlXeHNZbUZqYTNNPQwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAEM If the culprit above is wrong, please file a bug using this link and hit submit: https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Analyzer%20-%20Wrong%20result%20for%20FocusManagerTest.ViewFocusCallbacks&comment=Link%20to%20Analysis%3A%20https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVypgELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJwY2hyb21pdW0uY2hyb21pdW1vcy9saW51eC1jaHJvbWVvcy1kYmcvNzE4MS92aWV3c19tdXNfdW5pdHRlc3RzL1JtOWpkWE5OWVc1aFoyVnlWR1Z6ZEM1V2FXVjNSbTlqZFhORFlXeHNZbUZqYTNNPQwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAEM Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
,
Aug 9
My patch was reverted, so I'm moving this to fixed. Will ensure next landing doesn't reintroduce flake.
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bef3de4ccd2c52deb936d39dd3a31d86394c5906 commit bef3de4ccd2c52deb936d39dd3a31d86394c5906 Author: Scott Violet <sky@chromium.org> Date: Thu Aug 09 23:11:26 2018 reland: window-service: ignore requests to focus null This differs from the first version in that SetFocus(null) now returns true. As you suggested in the last patch, returning false caused the client to attempt to revert, which lead to some oddities. So, it seems safer to return true. views_mus_unittests no longer fails with this version, and https://chromium-review.googlesource.com/c/chromium/src/+/1169525 fixs the issue with the browser_test NavigatingExtensionPopupBrowserTest.Webpage. Here's my comment as to why this is ignored: // The client is asking to remove focus from a window. This is typically a // side effect of the window becoming, or about to become, an unfocusable // Window (for example, the Window is hiding). Windows becoming unfocusable is // handled locally. Assume the request is for such a scenario and return // true. Returning false means the client will attempt to revert to the // previously focused window, which may cause unexpected activation changes. // // To process null requests conflicts with top-level activation changes. For // example, the typical sequence when a window is hidden is to first remove // focus, and then hide the window. FocusController keys off window hiding to // move activation. If this code were to set focus to null, FocusController // would not see the window hiding (because the active window was set to null) // and not automatically activate the next window. // // Another possibility for this code is to handle null as a signal to move // focus to the active window (if there is one). I'm going with the simpler // approach for now. BUG= 867654 , 871652 TEST=covered by tests Change-Id: Iff2ddb31c2b99b977bca8e56fb653b6ce4eb6813 Reviewed-on: https://chromium-review.googlesource.com/1165692 Commit-Queue: Scott Violet <sky@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#581951} [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/ash/main.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/ash/shell/content/client/shell_main_delegate.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/services/ui/ws2/focus_handler.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/services/ui/ws2/focus_handler_unittest.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/ui/views/focus/focus_manager_unittest.cc |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by Findit
, Aug 7