Re-enable select_file_dialog_extension_browsertest suite |
||||||||||||||
Issue descriptionFixing Files app extension dialog issue 453634 , noticed [1] the dialog extension browser tests chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc are disabled on Chrome OS. [1] https://chromium-review.googlesource.com/c/chromium/src/+/1280383#message-3369b43a9740e06f852a4c8e44a0d45b117d1b3d We should work out why the tests are failing. The tests were blanket disabled in code revision: https://chromium.googlesource.com/chromium/src/+/24e0842a2a9d8f5aaacabc4a3ea94b3c22995e31%5E%21/#F0 under bug issue 477360 , then that issue was archived. Related bugs: Use "ready" message instead of "worker-initialized" message for test -- details of how the messages work. https://bugs.chromium.org/p/chromium/issues/detail?id=432029 SelectFileDialogExtensionBrowserTest.SelectFileAndCancel flaky on CrOS MSAN -- has a trace and fix for a different kind of flake https://bugs.chromium.org/p/chromium/issues/detail?id=453613
,
Oct 16
"under bug issue 477360 , then that issue was archived", that issue had a crash stack, and in it is a fatal error. [19446:19446:0415/082737:FATAL:private_api_file_system.cc(687)] Check failed: external_backend->CanHandleType(file_system_url.type()). #0 0x7fd32947c17e base::debug::StackTrace::StackTrace() #1 0x7fd3294d2cbf logging::LogMessage::~LogMessage() Now I wonder where we've seen that before (no prizes for guessing).
,
Oct 16
Answ: https://bugs.chromium.org/p/chromium/issues/detail?id=804413 ... which was one symptom of the great File Manager Browser Test flakiness problem: issue 831074 issue 804413 issue 829310 Now we see issue 477360 as evidence that that test flakiness problem existed as far back as Apr 15, 2015.
,
Oct 16
,
Oct 16
,
Oct 17
The systemic flake in these tests is caused by the same issue identified and fixed the File Manager browser tests: crbug.com/831074#c11 "If you want to use a resource during testing, then ensuring that resource is ready, before the app under test starts and looks for it, and before testing even begins, is the well-known recipe for success". Without that, systemic flakiness results. These browser tests in the SetUp() phase call: extensions::ComponentLoader::EnableBackgroundExtensionsForTesting(); which allows the FilesApp extensions to start, but only later do they setup the needed "Downloads" mount point resource FilesApp will look for. The causes a race. If FilesApp wins the race, it tries to access "Downloads", which is not yet setup, and hits a CHECK stop crash [19446:19446:0415/082737:FATAL:private_api_file_system.cc(687)] Check failed: external_backend->CanHandleType(file_system_url.type()). #0 0x7fd32947c17e base::debug::StackTrace::StackTrace() #1 0x7fd3294d2cbf logging::LogMessage::~LogMessage()
,
Oct 17
,
Oct 17
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/684ff899d4dc1152888bdbf5604859b5d2898d73 commit 684ff899d4dc1152888bdbf5604859b5d2898d73 Author: Noel Gordon <noel@chromium.org> Date: Wed Oct 17 03:45:05 2018 Deflake the SelectFileDialogExtensionBrowsertest setup code Setup the "Downloads" directory in a SetUpOnMainThread step, where the required profile() first becomes available. "Downloads" is a required resource for FilesApp so enable the FilesApp component extensions _after_ it has been created. Doing the reverse as in the original code caused a data race accessing "Downloads", and the result was systemic test flakiness ( issue 477360 ). Other code health and clean-up: - remove AddMountPoint() not needed anymore: "Downloads" setup is now handled by the SetUpOnMainThread step. - tune code comments make them precise and up-to-date: 'selection-change-complete' was mentioned, but was removed in later refactoring CLs. - add future TODO need to fix a FilesApp issue with CheckJavascriptErrors. - base::ScopedTempDir tmp_dir_ make it the first member of the helper class, so it's dtor() is called last. Life on the bots is more robust that way. No change in behavior, no new tests: disabled tests will be re-enabled in follow-up changes (so the chrome sheriff has an easier job). Bug: 895703 Change-Id: Ib75ee7163ecc3aec62c474f9834ab839ae879dfd Reviewed-on: https://chromium-review.googlesource.com/c/1285830 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#600267} [modify] https://crrev.com/684ff899d4dc1152888bdbf5604859b5d2898d73/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bfc307720e6bdb2607ddd59a1dc04abadfd49e39 commit bfc307720e6bdb2607ddd59a1dc04abadfd49e39 Author: Noel Gordon <noel@chromium.org> Date: Wed Oct 17 05:25:21 2018 Re-enable SelectFileDialogExtensionBrowserTest/SelectFileAndCancel Bug: 895703 Change-Id: I529f7a310c4323cc5a3d87a978e9cbcfe0049fe2 Reviewed-on: https://chromium-review.googlesource.com/c/1285491 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#600287} [modify] https://crrev.com/bfc307720e6bdb2607ddd59a1dc04abadfd49e39/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee6a4863d99cdffbb2e90ddee81982ab2b8ac69e commit ee6a4863d99cdffbb2e90ddee81982ab2b8ac69e Author: Noel Gordon <noel@chromium.org> Date: Wed Oct 17 05:28:28 2018 Re-enable SelectFileDialogExtensionBrowserTest/SelectFileAndOpen No-presubmit: true Bug: 895703 Change-Id: I9c56865d92ecf67292c8adc4d41d72b1d5ae1aec Reviewed-on: https://chromium-review.googlesource.com/c/1285869 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#600288} [modify] https://crrev.com/ee6a4863d99cdffbb2e90ddee81982ab2b8ac69e/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b905d48009cdaebc6bf69d605c7e8778666b5aeb commit b905d48009cdaebc6bf69d605c7e8778666b5aeb Author: Noel Gordon <noel@chromium.org> Date: Wed Oct 17 05:31:56 2018 Re-enable SelectFileDialogExtensionBrowserTest/SelectFileAndSave No-presubmit: true Bug: 895703 Change-Id: Ib7996d7f75e2c7bda08bf7ac01b45288e8fb022a Reviewed-on: https://chromium-review.googlesource.com/c/1285889 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#600290} [modify] https://crrev.com/b905d48009cdaebc6bf69d605c7e8778666b5aeb/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/025b8fc8b1d07731337e3d7915b7d11c0b758ed1 commit 025b8fc8b1d07731337e3d7915b7d11c0b758ed1 Author: Noel Gordon <noel@chromium.org> Date: Wed Oct 17 06:27:23 2018 Re-enable SelectFileDialogExtensionBrowserTest/OpenSingletonTabAndCancel No-presubmit: true Bug: 895703 Change-Id: If777a1a20b66411d99a5655b72c9a1b957670201 Reviewed-on: https://chromium-review.googlesource.com/c/1285909 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#600300} [modify] https://crrev.com/025b8fc8b1d07731337e3d7915b7d11c0b758ed1/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8eed5bfe62aef636d8c5cd1c16603b5957002d72 commit 8eed5bfe62aef636d8c5cd1c16603b5957002d72 Author: Noel Gordon <noel@chromium.org> Date: Wed Oct 17 06:28:53 2018 Re-enable SelectFileDialogExtensionBrowserTest/OpenTwoDialogs No-presubmit: true Bug: 895703 Change-Id: Ia8322806f4c2b584536065f35429466ab0a22ba4 Reviewed-on: https://chromium-review.googlesource.com/c/1285254 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#600301} [modify] https://crrev.com/8eed5bfe62aef636d8c5cd1c16603b5957002d72/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
,
Oct 17
All tests green on all bots [1], no flakes. https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=SelectFileDialogExtensionBrowserTest
,
Oct 17
,
Oct 19
With this suite re-enabled, we now have another test group that can help address issue 879434, being the complete lack of ash virtual keyboard tests in FilesApp. Bots #15 looking all-green today as well: verified fixed.
,
Nov 12
Re-opening to land a TODO fix. Test opening FilesApp file dialog from a web page.
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/84188d7c02dcc31d16fc32e55bf9a3336576ea2f commit 84188d7c02dcc31d16fc32e55bf9a3336576ea2f Author: Noel Gordon <noel@chromium.org> Date: Tue Nov 13 03:26:55 2018 Test opening the extension dialog from web page file <input> element FilesApp provides the innards of the extension file dialog. Add a test to open the file dialog by clicking on an <input type=file> element in a web page. Bug: 895703 Change-Id: Id6f56e2df4034af66a57f888195bc841a55da2b4 Reviewed-on: https://chromium-review.googlesource.com/c/1331099 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Commit-Position: refs/heads/master@{#607470} [modify] https://crrev.com/84188d7c02dcc31d16fc32e55bf9a3336576ea2f/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc [add] https://crrev.com/84188d7c02dcc31d16fc32e55bf9a3336576ea2f/chrome/test/data/chromeos/file_manager/file_input/element.html
,
Nov 13
,
Nov 15
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b764625cfe5f18e10b3e5bca0267fdcb0751031 commit 8b764625cfe5f18e10b3e5bca0267fdcb0751031 Author: Noel Gordon <noel@chromium.org> Date: Thu Nov 15 01:49:22 2018 Add FilesApp owners for select_file_dialog_extension FilesApp OWNERS should be consulted, and also help review changes, to the ChromeOS system modal select file dialog. Bug: 895703 Change-Id: I4258de9c2b3bfc7e37e68b546c9e16a1395572dc Reviewed-on: https://chromium-review.googlesource.com/c/1336952 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#608216} [modify] https://crrev.com/8b764625cfe5f18e10b3e5bca0267fdcb0751031/chrome/browser/ui/views/OWNERS
,
Nov 15
,
Dec 5
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5ffc039907a8f6d9bb8db8b072d6c8d45c208ca0 commit 5ffc039907a8f6d9bb8db8b072d6c8d45c208ca0 Author: Noel Gordon <noel@chromium.org> Date: Wed Dec 05 03:28:24 2018 select_file_dialog_extension_browsertest: assert owning_window ASSERT owning_window state in each test case. The recently added test, see CL:1337144, added a new test case for when there is no window that owns the dialog (good). The test was restricted to CHROME_OS, but this entire browsertest suite is CHROME_OS only, so no need to restrict. Test: --gtest_filter="SelectFileDialogExtensionBrowserTest*" Tbr: lucmult-away@ Bug: 895703 Change-Id: I9b201505e0ced91c96885f5d1868a4a98e1b2007 Reviewed-on: https://chromium-review.googlesource.com/c/1361756 Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#613847} [modify] https://crrev.com/5ffc039907a8f6d9bb8db8b072d6c8d45c208ca0/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc
,
Dec 5
|
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by noel@chromium.org
, Oct 16Labels: CrOSFilesCategory-Testing OS-Chrome
Status: Available (was: Untriaged)