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

Issue 296003014: Revert 272217 "LanguageState should be owned by TranslateManager" (Closed)

Created:
6 years, 7 months ago by sadrul
Modified:
6 years, 7 months ago
Reviewers:
nshaik
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 272217 "LanguageState should be owned by TranslateManager" This CL is breaking the asan/lsan bots. A snippet of the failure: ==10649==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b0000d0592 at pc 0xbf31f90 bp 0x7fff0930b610 sp 0x7fff0930b608 WRITE of size 1 at 0x60b0000d0592 thread T0 (browser_tests) #0 0xbf31f8f in set_translation_declined components/translate/core/browser/language_state.h:62 #1 0xbf31f8f in TranslateUIDelegate::TranslationDeclined(bool) components/translate/core/browser/translate_ui_delegate.cc:188 #2 0x4614bb4 in TranslateBubbleView::WindowClosing() chrome/browser/ui/views/translate/translate_bubble_view.cc:205 #3 0x4224e9c in views::Widget::OnNativeWidgetDestroying() ui/views/widget/widget.cc:1086 #4 0x4212c95 in OnWindowDestroying ui/views/widget/native_widget_aura.cc:793 #5 0x4212c95 in non-virtual thunk to views::NativeWidgetAura::OnWindowDestroying(aura::Window*) ui/views/widget/native_widget_aura.cc:797 #6 0x60362b7 in aura::Window::~Window() ui/aura/window.cc:225 #7 0x60388ed in aura::Window::~Window() ui/aura/window.cc:218 #8 0x421fb19 in views::Widget::CloseNow() ui/views/widget/widget.cc:614 #9 0x5a03a46 in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:422 #10 0x5a05ead in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:373 #11 0x443bda1 in TabStripModel::InternalCloseTab(content::WebContents*, int, bool) chrome/browser/ui/tabs/tab_strip_model.cc:1272 #12 0x4434198 in TabStripModel::InternalCloseTabs(std::vector\u003Cint, std::allocator\u003Cint> > const&, unsigned int) chrome/browser/ui/tabs/tab_strip_model.cc:1247 #13 0x4432c73 in TabStripModel::CloseAllTabs() chrome/browser/ui/tabs/tab_strip_model.cc:545 ... 0x60b0000d0592 is located 82 bytes inside of 104-byte region [0x60b0000d0540,0x60b0000d05a8) freed by thread T0 (browser_tests) here: #0 0x56fcbb in operator delete(void*) /usr/local/google/work/chromium/src/third_party/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:94 #1 0xbf20b7c in TranslateManager::~TranslateManager() components/translate/core/browser/translate_manager.cc:59 #2 0x2926a06 in operator() base/memory/scoped_ptr.h:137 #3 0x2926a06 in reset base/memory/scoped_ptr.h:246 #4 0x2926a06 in reset base/memory/scoped_ptr.h:367 #5 0x2926a06 in WebContentsDestroyed chrome/browser/translate/translate_tab_helper.cc:314 #6 0x2926a06 in non-virtual thunk to TranslateTabHelper::WebContentsDestroyed() chrome/browser/translate/translate_tab_helper.cc:315 #7 0x5a03a46 in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:422 #8 0x5a05ead in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:373 #9 0x443bda1 in TabStripModel::InternalCloseTab(content::WebContents*, int, bool) chrome/browser/ui/tabs/tab_strip_model.cc:1272 #10 0x4434198 in TabStripModel::InternalCloseTabs(std::vector\u003Cint, std::allocator\u003Cint> > const&, unsigned int) chrome/browser/ui/tabs/tab_strip_model.cc:1247 #11 0x4432c73 in TabStripModel::CloseAllTabs() chrome/browser/ui/tabs/tab_strip_model.cc:545 ... previously allocated by thread T0 (browser_tests) here: #0 0x56f77b in operator new(unsigned long) /usr/local/google/work/chromium/src/third_party/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:62 #1 0x2923028 in TranslateTabHelper::TranslateTabHelper(content::WebContents*) chrome/browser/translate/translate_tab_helper.cc:78 #2 0x4428959 in CreateForWebContents content/public/browser/web_contents_user_data.h:38 #3 0x4428959 in TabHelpers::AttachTabHelpers(content::WebContents*) chrome/browser/ui/tab_helpers.cc:142 ... > LanguageState should be owned by TranslateManager > > LanguageState is currently owned by ContentTranslateManager, > but it should be moved under TranslateManager > > BUG=345690 > TEST=unittests --gtest_filter=Translate* > TBR=thakis@chromium.org > > Review URL: https://codereview.chromium.org/290573013 TBR=naiem.shaik@gmail.com Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272264

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -48 lines) Patch
M trunk/src/chrome/browser/extensions/api/tabs/tabs_api.cc View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/policy/policy_browsertest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/translate/translate_infobar_delegate.cc View 1 chunk +2 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/translate/translate_tab_helper.h View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/translate/translate_tab_helper.cc View 8 chunks +7 lines, -12 lines 0 comments Download
M trunk/src/chrome/browser/ui/browser_browsertest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/ui/browser_commands.cc View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/ui/views/frame/browser_view.cc View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/chrome/browser/ui/views/location_bar/location_bar_view.cc View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/components/translate/content/browser/content_translate_driver.h View 3 chunks +8 lines, -1 line 0 comments Download
M trunk/src/components/translate/content/browser/content_translate_driver.cc View 3 chunks +15 lines, -0 lines 0 comments Download
M trunk/src/components/translate/core/browser/language_state_unittest.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M trunk/src/components/translate/core/browser/translate_driver.h View 2 chunks +4 lines, -0 lines 0 comments Download
M trunk/src/components/translate/core/browser/translate_manager.h View 3 chunks +0 lines, -6 lines 0 comments Download
M trunk/src/components/translate/core/browser/translate_manager.cc View 7 chunks +12 lines, -16 lines 0 comments Download
M trunk/src/components/translate/core/browser/translate_ui_delegate.cc View 4 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
sadrul
6 years, 7 months ago (2014-05-22 19:07:32 UTC) #1
sadrul
6 years, 7 months ago (2014-05-22 19:10:35 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r272264.

Powered by Google App Engine
This is Rietveld 408576698