Fix (mostly Cocoa) tests that fail on Mac when ShowAllDialogsWithViewsToolkit is enabled |
|||
Issue descriptionChrome Version : 64 OS Version: OS X 10.12.6 A lot of these are failing just because the Cocoa UI/window isn't being created, so there's nothing to test. Fix is largely going to be a ScopedFeatureList that disables SecondaryUiMd when testing specifically Cocoa UI. But there may be some things not specifically testing Cocoa UI that need extra coverage. bot run: https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/578638 failing tests: browser_tests: BrowserWindowControllerTest.InfoBarTipStretchedWhenBookmarkBarStatusChanged ContentSettingDecorationTest.DecorationPress ExtensionMessageBubbleBrowserTestMac.* (12) FirstRunMasterPrefsImportNothing.ImportNothingAndShowNewTabPage FirstRunMasterPrefsWithTrackedPreferencesInstance/FirstRunMasterPrefsWithTrackedPreferences.TrackedPreferencesSurviveFirstRun/0 FirstRunMasterPrefsWithTrackedPreferencesInstance/FirstRunMasterPrefsWithTrackedPreferences.TrackedPreferencesSurviveFirstRun/1 ManagePasswordsBubbleTest.* (4) NtpBubbleBrowserTestMac.* (2) ProfileSigninConfirmationHelperBrowserTest.* (2) StartupBrowserCreatorFirstRunTest.* (2) interactive_ui_tests: AutofillInteractiveTest.AutofillAfterTranslate native_theme_unittests: NativeThemeMacTest.SystemColorSpotChecks unit_tests: BookmarkBubbleControllerTest.TestBubbleWindow ManagePasswordsBubbleCocoaTest.* (8) PermissionSelectorButtonTest.* (3) TranslateBubbleControllerTest.* (4)
,
Nov 1 2017
First pass https://chromium-review.googlesource.com/c/chromium/src/+/748487 should fix unit_tests: BookmarkBubbleControllerTest.TestBubbleWindow ManagePasswordsBubbleCocoaTest.* (8) PermissionSelectorButtonTest.* (3) TranslateBubbleControllerTest.* (4) and browser_tests: ManagePasswordsBubbleTest.* (4) ExtensionMessageBubbleBrowserTestMac.* (12) remaining (feel free to grab any and update this bug..): BrowserWindowControllerTest.InfoBarTipStretchedWhenBookmarkBarStatusChanged - might actually be a bug ContentSettingDecorationTest.DecorationPress - I think we want ContentSettingBubbleViewsBridge::Show(..) to return a gfx::NativeWindow so that content_setting_decoration.mm can store it. Annoying it's just for a test though. this bunch: FirstRun* NtpBubbleBrowserTestMac.* (2) ProfileSigninConfirmationHelperBrowserTest.* (2) StartupBrowserCreatorFirstRunTest.* (2) AutofillInteractiveTest.AutofillAfterTranslate - I think these are creating random/unowned/unparented windows. Aura relies on some code in application_lifetime_aura that calls views::Widget::CloseAllSecondaryWidgets(). I think we need to call that on Mac as well. (which also requires implementing it!) native_theme_unittests: NativeThemeMacTest.SystemColorSpotChecks - haven't looked at this yet, but it's probably a matter of changing the things that are spot-checked in the test
,
Nov 1 2017
,
Nov 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5da19f1b92ef9faa281ba146b15338aa2105ed29 commit 5da19f1b92ef9faa281ba146b15338aa2105ed29 Author: Trent Apted <tapted@chromium.org> Date: Wed Nov 01 22:46:38 2017 Disable SecondaryUiMd in some tests that only test Cocoa UI. These test harnesses specifically test Cocoa dialogs that are not shown when SecondaryUiMd is enabled. Bug: 780409 Change-Id: Ibdce953bad102464bed17b998c4fcabdf64ace53 Reviewed-on: https://chromium-review.googlesource.com/748487 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513306} [modify] https://crrev.com/5da19f1b92ef9faa281ba146b15338aa2105ed29/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm [modify] https://crrev.com/5da19f1b92ef9faa281ba146b15338aa2105ed29/chrome/browser/ui/cocoa/extensions/extension_message_bubble_browsertest_mac.mm [modify] https://crrev.com/5da19f1b92ef9faa281ba146b15338aa2105ed29/chrome/browser/ui/cocoa/page_info/permission_selector_button_unittest.mm [modify] https://crrev.com/5da19f1b92ef9faa281ba146b15338aa2105ed29/chrome/browser/ui/cocoa/passwords/passwords_bubble_browsertest.mm [modify] https://crrev.com/5da19f1b92ef9faa281ba146b15338aa2105ed29/chrome/browser/ui/cocoa/passwords/passwords_bubble_cocoa_unittest.mm [modify] https://crrev.com/5da19f1b92ef9faa281ba146b15338aa2105ed29/chrome/browser/ui/cocoa/translate/translate_bubble_controller_unittest.mm
,
Nov 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ecd252f35b6e8671d0a3e387ea8ad246859ecaee commit ecd252f35b6e8671d0a3e387ea8ad246859ecaee Author: Justin Donnelly <jdonnelly@chromium.org> Date: Thu Nov 02 01:01:34 2017 Revert "Disable SecondaryUiMd in some tests that only test Cocoa UI." This reverts commit 5da19f1b92ef9faa281ba146b15338aa2105ed29. Reason for revert: Reverting on suspicion of breaking ExtensionMessageBubbleBrowserTestMac.TestClickingLearnMoreButton and other bubble tests on several Mac bots. See, for example: https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.12%20Tests/builds/6896 -- Chromium sheriff Original change's description: > Disable SecondaryUiMd in some tests that only test Cocoa UI. > > These test harnesses specifically test Cocoa dialogs that are not shown > when SecondaryUiMd is enabled. > > Bug: 780409 > Change-Id: Ibdce953bad102464bed17b998c4fcabdf64ace53 > Reviewed-on: https://chromium-review.googlesource.com/748487 > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> > Commit-Queue: Trent Apted <tapted@chromium.org> > Cr-Commit-Position: refs/heads/master@{#513306} TBR=ellyjones@chromium.org,tapted@chromium.org Change-Id: I75466a2200968ce45b018dd9558191bafe98288c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 780409 Reviewed-on: https://chromium-review.googlesource.com/749853 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#513351} [modify] https://crrev.com/ecd252f35b6e8671d0a3e387ea8ad246859ecaee/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm [modify] https://crrev.com/ecd252f35b6e8671d0a3e387ea8ad246859ecaee/chrome/browser/ui/cocoa/extensions/extension_message_bubble_browsertest_mac.mm [modify] https://crrev.com/ecd252f35b6e8671d0a3e387ea8ad246859ecaee/chrome/browser/ui/cocoa/page_info/permission_selector_button_unittest.mm [modify] https://crrev.com/ecd252f35b6e8671d0a3e387ea8ad246859ecaee/chrome/browser/ui/cocoa/passwords/passwords_bubble_browsertest.mm [modify] https://crrev.com/ecd252f35b6e8671d0a3e387ea8ad246859ecaee/chrome/browser/ui/cocoa/passwords/passwords_bubble_cocoa_unittest.mm [modify] https://crrev.com/ecd252f35b6e8671d0a3e387ea8ad246859ecaee/chrome/browser/ui/cocoa/translate/translate_bubble_controller_unittest.mm
,
Nov 3 2017
OK! https://chromium-review.googlesource.com/c/chromium/src/+/750601 - tests in c/b/ui/cocoa that only care about cocoa code https://chromium-review.googlesource.com/c/chromium/src/+/752782 - BrowserWindowControllerTest.InfoBarTipStretchedWhenBookmarkBarStatusChanged https://chromium-review.googlesource.com/c/chromium/src/+/752905 - ContentSettingDecorationTest.DecorationPress https://chromium-review.googlesource.com/c/chromium/src/+/753061 - NativeThemeMacTest.SystemColorSpotChecks https://chromium-review.googlesource.com/c/chromium/src/+/752902 - a bunch of things wanting views::Widget::CloseAllSecondaryWidgets() https://chromium-review.googlesource.com/c/chromium/src/+/750444 - CL with jobs flipping the flag on Mac!
,
Nov 3 2017
Oh! There's one more - AutofillInteractiveTest.AutofillAfterTranslate .. I haven't got it working yet. There's a weird renderer crash at translate_helper.cc(336) when it checks for TranslateHelper::IsTranslateLibAvailable() WIP: https://chromium-review.googlesource.com/752783 possible alternative to get the test passing -> https://chromium-review.googlesource.com/752908
,
Nov 3 2017
yah - AutofillAfterTranslate is the only error in the trial flag flip at https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/580652
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/187e23a1b6de89d7a2b79b4f47c3bfaafa1b59ec commit 187e23a1b6de89d7a2b79b4f47c3bfaafa1b59ec Author: Trent Apted <tapted@chromium.org> Date: Fri Nov 03 21:22:11 2017 Skip AutofillInteractiveTest.AutofillAfterTranslate on MacViews The test flips ::switches::kEnableTranslateNewUX which is never going to launch on Mac as Cocoa UI. On MacViews, kEnableTranslateNewUX will engage the MacViews bubble UI which isn't ready and isn't used yet. The Cocoa browser still uses an InfoBar, even with MacViews/Harmony. See http://crbug.com/781134 . So only continue when testing the Cocoa UI (i.e. the one that won't launch). Bug: 780409 , 781134 Change-Id: Iaf5aa156f687578e632894f80c9ce6a9b57ccc03 Reviewed-on: https://chromium-review.googlesource.com/752908 Reviewed-by: Mathieu Perreault <mathp@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513914} [modify] https://crrev.com/187e23a1b6de89d7a2b79b4f47c3bfaafa1b59ec/chrome/browser/autofill/autofill_interactive_uitest.cc
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d2d5ce69bbc0c4cb07204aed7dad82ace633eee commit 1d2d5ce69bbc0c4cb07204aed7dad82ace633eee Author: Trent Apted <tapted@chromium.org> Date: Fri Nov 03 21:48:44 2017 Reland: Disable SecondaryUiMd in some tests that only test Cocoa UI. These test harnesses specifically test Cocoa dialogs that are not shown when SecondaryUiMd is enabled. First attempt suffered from a subtle trap in SetUp() ordering when using feature lists in browser_tests (crbug/722409). Bug: 780409 Change-Id: I5129e8b6a76e3b60b1ea6f32fcabd2804bdb6cb9 Reviewed-on: https://chromium-review.googlesource.com/750601 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513923} [modify] https://crrev.com/1d2d5ce69bbc0c4cb07204aed7dad82ace633eee/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm [modify] https://crrev.com/1d2d5ce69bbc0c4cb07204aed7dad82ace633eee/chrome/browser/ui/cocoa/extensions/extension_message_bubble_browsertest_mac.mm [modify] https://crrev.com/1d2d5ce69bbc0c4cb07204aed7dad82ace633eee/chrome/browser/ui/cocoa/page_info/permission_selector_button_unittest.mm [modify] https://crrev.com/1d2d5ce69bbc0c4cb07204aed7dad82ace633eee/chrome/browser/ui/cocoa/passwords/passwords_bubble_browsertest.mm [modify] https://crrev.com/1d2d5ce69bbc0c4cb07204aed7dad82ace633eee/chrome/browser/ui/cocoa/passwords/passwords_bubble_cocoa_unittest.mm [modify] https://crrev.com/1d2d5ce69bbc0c4cb07204aed7dad82ace633eee/chrome/browser/ui/cocoa/translate/translate_bubble_controller_unittest.mm
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49bbbf39fc02d2c4fbe41f7e30b685e05b77d85d commit 49bbbf39fc02d2c4fbe41f7e30b685e05b77d85d Author: Trent Apted <tapted@chromium.org> Date: Fri Nov 03 22:03:16 2017 Remove NativeThemeMacTest.SystemColorSpotChecks This is more of a "change detector" test that isn't adding much value. It's fragile to macOS changes out of our control, and breaks under Harmony. The particular constant it checks, kColorId_WindowBackground, is only used by the IME window on ChromeOS. The test now fails because Harmony adopts a consistent dialog background color across all platforms and OS versions; kColorId_WindowBackground is fixed. There are other system colors that Harmony does adopt, but they are configurable in System Preferences, so checking them may cause a test to fail on developer machines or misconfigured bots. Bug: 780409 Change-Id: I374dad77fb00fbe5ebcc772b97e8acd3fb768564 Reviewed-on: https://chromium-review.googlesource.com/753061 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513924} [modify] https://crrev.com/49bbbf39fc02d2c4fbe41f7e30b685e05b77d85d/ui/native_theme/native_theme_mac_unittest.cc
,
Nov 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a7026b7f4a0665a8f7bdb17e4050dd98511f05e9 commit a7026b7f4a0665a8f7bdb17e4050dd98511f05e9 Author: Trent Apted <tapted@chromium.org> Date: Fri Nov 03 23:01:55 2017 MacViews: Implement Widget::CloseAllSecondaryWidgets() Views Widgets host ui::Compositors that talk to the GPU process, whose host complains if it is destroyed while in-use. By this point, all browser windows are closed, which tear down any Widgets parented to them. This will additionally close any unparented, non-Browser Widgets. Test: WidgetTest.CloseAllSecondaryWidgets Bug: 780409 Change-Id: Ieea01134a843abb04816c82933f3a3a301cd2647 Reviewed-on: https://chromium-review.googlesource.com/752902 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513932} [modify] https://crrev.com/a7026b7f4a0665a8f7bdb17e4050dd98511f05e9/chrome/browser/lifetime/application_lifetime_mac.mm [modify] https://crrev.com/a7026b7f4a0665a8f7bdb17e4050dd98511f05e9/ui/views/widget/native_widget_mac.mm [modify] https://crrev.com/a7026b7f4a0665a8f7bdb17e4050dd98511f05e9/ui/views/widget/widget_unittest.cc
,
Nov 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b03bc3afcf0dbf003eb3989f21e7a42e87697b10 commit b03bc3afcf0dbf003eb3989f21e7a42e87697b10 Author: Trent Apted <tapted@chromium.org> Date: Fri Nov 03 23:02:06 2017 MacViews: Fix ContentSettingDecorationTest.DecorationPress with SecondaryUiMd The ContentSettingDecoration is in the browser, so still used in MacViews. Follow the pattern used by the Cocoa bubble, and retain a window. This change also reduces the cases where a bubble reopens (rather than dismissing) when clicking the decoration a second time. However, there's an orthogonal issue where this may still occur depending on the order of events (the views bubble dismisses on focus loss, so may be hidden by the time the mouse event is processed on the decoration). It's also a bit weird that the NSWindow is retained for the lifetime of the decoration, but that's what Cocoa does, and it doesn't matter for views so long as the window isn't reshown. Bug: 780409 Change-Id: I7d46904d3c1ae8f2dd482537126908cf00389fd9 Reviewed-on: https://chromium-review.googlesource.com/752905 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513933} [modify] https://crrev.com/b03bc3afcf0dbf003eb3989f21e7a42e87697b10/chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc [modify] https://crrev.com/b03bc3afcf0dbf003eb3989f21e7a42e87697b10/chrome/browser/ui/cocoa/browser_dialogs_views_mac.h [modify] https://crrev.com/b03bc3afcf0dbf003eb3989f21e7a42e87697b10/chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm
,
Nov 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d64d9d0dacfaee4e4ddb209307dcfdc6f628b3e commit 1d64d9d0dacfaee4e4ddb209307dcfdc6f628b3e Author: Trent Apted <tapted@chromium.org> Date: Fri Nov 03 23:03:13 2017 Fix BrowserWindowControllerTest.InfoBarTipStretchedWhenBookmarkBarStatusChanged Infobars used to anchor from the same point as PageInfo. Now they don't. I renamed the method used in the browser code, but the test file still tests expectations against the old name. Bug: 780409 Change-Id: I2b1130bdd637719cc51f9a642710061bbc2c37c1 Reviewed-on: https://chromium-review.googlesource.com/752782 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513935} [modify] https://crrev.com/1d64d9d0dacfaee4e4ddb209307dcfdc6f628b3e/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
,
Nov 7 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by tapted@chromium.org
, Nov 1 2017