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

Issue 346044: Fix for flaky ExtensionBrowserTest.Popup test (Closed)

Created:
11 years, 1 month ago by jeff.timanus
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com, Aaron Boodman, Erik does not do reviews, ben+cc_chromium.org, pam+watch_chromium.org, rafaelw
Visibility:
Public.

Description

Correction of race-condition in ExtensionBrowserTest::WaitForExtensionHostsToLoad() that was causing flakiness in the ExtensionBrowserTest.Popup test. The issue is that with the addition of the chrome.experimental.popup.* set of API's, we are now creating new ExtensionHosts programatically through JS. The existing test harness was not designed for this behaviour, and was iterating through a container assuming that container was immutable. I also re-enabled ExtensionBrowserTest.Popup, the previously disabled test. BUG=26599 TEST=extension_popup_apitest.cc committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30982

Patch Set 1 #

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -8 lines) Patch
M chrome/browser/extensions/extension_browsertest.cc View 2 chunks +8 lines, -5 lines 0 comments Download
M chrome/browser/extensions/extension_popup_apitest.cc View 1 chunk +1 line, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
jeff.timanus
Hi Guys, I found the issue with the flaky Popup test. It turned out to ...
11 years, 1 month ago (2009-11-03 22:54:02 UTC) #1
jeff.timanus
11 years, 1 month ago (2009-11-03 22:57:52 UTC) #2
Erik does not do reviews
LGTM
11 years, 1 month ago (2009-11-04 00:17:14 UTC) #3
Paweł Hajdan Jr.
11 years, 1 month ago (2009-11-04 06:23:36 UTC) #4
Drive-by. Thanks for fixing the flakiness! LGTM.

Tiny-nit: the BUG line in the commit message should be just BUG=26599 (without
the crbug.com part). It will get linkified in Rietveld. I edited the issue to
fix that.

Powered by Google App Engine
This is Rietveld 408576698