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

Issue 2879004: Revert 50913... (Closed)

Created:
10 years, 6 months ago by Jay Civelli
Modified:
9 years, 7 months ago
Reviewers:
Jay Civelli
CC:
chromium-reviews, Paweł Hajdan Jr., ben+cc_chromium.org
Visibility:
Public.

Description

Revert 50913 TranslateManagerTest.NormalTranslate fails on Linux TranslateInfoBarDelegate2 were leaked. (Yikes!) This revealed that the NormalTranslate unit-test was incorrect (it was not testing that changing the source or target language from the after translate infobar causes a translation). This also revealed that returning reference string in the delegate is not a brilliant idea as when we switch infobars, the new infobar might be using a string from the old infobar that might have been deleted. BUG=None TEST=Linux heap check bot should be green. Review URL: http://codereview.chromium.org/2822032 TBR=jcivelli@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50914

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -42 lines) Patch
M chrome/browser/translate/translate_infobar_delegate2.h View 3 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/translate/translate_infobar_delegate2.cc View 3 chunks +6 lines, -10 lines 0 comments Download
M chrome/browser/translate/translate_manager2_unittest.cc View 1 chunk +7 lines, -26 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
Jay Civelli
10 years, 6 months ago (2010-06-26 00:28:13 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698