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

Issue 199091: Update ExtensionApiTest.Tabs and re-enable (Closed)

Created:
11 years, 3 months ago by rafaelw
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Update ExtensionApiTest.Tabs and re-enable BUG=20828 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=26002

Patch Set 1 #

Total comments: 9

Patch Set 2 : cr changes #

Total comments: 2

Patch Set 3 : remove comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -125 lines) Patch
M chrome/browser/extensions/extension_tabs_apitest.cc View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M chrome/renderer/renderer_resources.grd View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/resources/extension_apitest.js View 1 5 chunks +22 lines, -18 lines 0 comments Download
M chrome/test/data/extensions/api_test/tabs/test.js View 1 9 chunks +137 lines, -104 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
rafaelw
11 years, 3 months ago (2009-09-11 01:44:58 UTC) #1
asargent_no_longer_on_chrome
http://codereview.chromium.org/199091/diff/1/5 File chrome/test/data/extensions/api_test/tabs/test.js (right): http://codereview.chromium.org/199091/diff/1/5#newcode101 Line 101: nit: consider removing blank line http://codereview.chromium.org/199091/diff/1/5#newcode276 Line 276: ...
11 years, 3 months ago (2009-09-11 17:54:29 UTC) #2
Erik does not do reviews
http://codereview.chromium.org/199091/diff/1/4 File chrome/renderer/resources/extension_apitest.js (right): http://codereview.chromium.org/199091/diff/1/4#newcode179 Line 179: that.doneListening = function() { can't you just do ...
11 years, 3 months ago (2009-09-11 18:12:52 UTC) #3
rafaelw
I tried both variants that Erik suggested and neither works. I don't believe there's anyway ...
11 years, 3 months ago (2009-09-11 19:05:05 UTC) #4
asargent_no_longer_on_chrome
this seems ok to me
11 years, 3 months ago (2009-09-11 19:28:57 UTC) #5
Erik does not do reviews
LGTM
11 years, 3 months ago (2009-09-11 19:46:06 UTC) #6
Paweł Hajdan Jr.
Enforcing order of events seems Really Good, always. Good luck! http://codereview.chromium.org/199091/diff/4001/4002 File chrome/browser/extensions/extension_tabs_apitest.cc (right): http://codereview.chromium.org/199091/diff/4001/4002#newcode7 ...
11 years, 3 months ago (2009-09-11 20:09:06 UTC) #7
rafaelw
11 years, 3 months ago (2009-09-11 20:12:09 UTC) #8
http://codereview.chromium.org/199091/diff/4001/4002
File chrome/browser/extensions/extension_tabs_apitest.cc (right):

http://codereview.chromium.org/199091/diff/4001/4002#newcode7
Line 7: // Flaky, http://crbug.com/20828. Please consult phajdan.jr before
re-enabling.
On 2009/09/11 20:09:06, Paweł Hajdan Jr. wrote:
> Please remove this comment. If all goes well, it won't be needed.

Done.

Powered by Google App Engine
This is Rietveld 408576698