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

Issue 599803002: Mac: Deflake AppListServiceInteractiveTest.SwitchAppListProfilesDuringSearch (Closed)

Created:
6 years, 3 months ago by tapted
Modified:
6 years, 3 months ago
Reviewers:
jackhou1
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@20140923-AppList-OnShutdown
Project:
chromium
Visibility:
Public.

Description

Mac: Deflake AppListServiceInteractiveTest.SwitchAppListProfilesDuringSearch This is flaky because the app list ends up trying to read an ImageSkia on one of Cocoa's background threads. While NSView has a `canDrawConcurrently` function, AppKit is not calling it in the case that is making this test flaky. The documentation says, "The default is NO for most types of views.." [sic], and I've verified it is `NO` and tried overriding it, but it was just never called. However, having -[NSView canDraw] return `NO` seems to work fine. Returning NO when there is no message loop ensures a draw only occurs on a thread that Chrome created (not one of Cocoa's background threads), and doesn't raise any DEPS issues. BUG=417148, 415264 Committed: https://crrev.com/9a068829483acbd5290eedbdc8102d92b98ffab8 Cr-Commit-Position: refs/heads/master@{#296591}

Patch Set 1 #

Patch Set 2 : rebase to master with test hack for bots #

Patch Set 3 : fix base #

Patch Set 4 : rebase to master #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -11 lines) Patch
M chrome/browser/ui/app_list/app_list_service_interactive_uitest.cc View 2 3 1 chunk +4 lines, -11 lines 0 comments Download
M ui/app_list/cocoa/apps_search_results_controller.mm View 2 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
tapted
OK last one (phew!). I think this one was always flaky, just it's quite rare ...
6 years, 3 months ago (2014-09-24 08:12:16 UTC) #2
jackhou1
lgtm
6 years, 3 months ago (2014-09-24 09:23:27 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/599803002/60001
6 years, 3 months ago (2014-09-25 00:10:54 UTC) #5
commit-bot: I haz the power
Committed patchset #4 (id:60001) as e7b4da7601bd98ea0c5a5eb05c206fcd23fa7a68
6 years, 3 months ago (2014-09-25 00:33:00 UTC) #6
commit-bot: I haz the power
6 years, 3 months ago (2014-09-25 00:33:38 UTC) #7
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/9a068829483acbd5290eedbdc8102d92b98ffab8
Cr-Commit-Position: refs/heads/master@{#296591}

Powered by Google App Engine
This is Rietveld 408576698