New issue
Advanced search Search tips

Issue 795987 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocking:
issue 795725



Sign in to add a comment

MacViews: Check failed: bubble->parent_window() when showing the "new" translate UX

Project Member Reported by tapted@chromium.org, Dec 19 2017

Issue description

Chrome Version       : 65.0.3294.5
OS Version: OS X 10.12.6

With,
 chrome://flags/#enable-translate-new-ux enabled [not default]
 chrome://flags/#secondary-ui-md enabled [default]

Right-click a page and select "Translate"

[67401:775:1219/123806.312273:FATAL:bubble_anchor_helper_views.mm(95)] Check failed: bubble->parent_window().
0   libbase.dylib                       0x0000000111e45493 base::debug::StackTrace::StackTrace(unsigned long) + 739
1   libbase.dylib                       0x0000000111e4586d base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x0000000111e4090c base::debug::StackTrace::StackTrace() + 28
3   libbase.dylib                       0x0000000111faafae logging::LogMessage::~LogMessage() + 2062
4   libbase.dylib                       0x0000000111fa3a65 logging::LogMessage::~LogMessage() + 21
5   libchrome_dll.dylib                 0x000000012a1416a8 (anonymous namespace)::BubbleAnchorHelper::BubbleAnchorHelper(views::BubbleDialogDelegateView*, LocationBarDecoration*, (anonymous namespace)::AnchorType) + 1816
6   libchrome_dll.dylib                 0x000000012a140f0b (anonymous namespace)::BubbleAnchorHelper::BubbleAnchorHelper(views::BubbleDialogDelegateView*, LocationBarDecoration*, (anonymous namespace)::AnchorType) + 43
7   libchrome_dll.dylib                 0x000000012a140eb0 KeepBubbleAnchored(views::BubbleDialogDelegateView*, LocationBarDecoration*) + 64
8   libchrome_dll.dylib                 0x000000012a6c8fd0 ShowTranslateBubbleViews(NSWindow*, LocationBarViewMac*, content::WebContents*, translate::TranslateStep, translate::TranslateErrors::Type, bool) + 1216
9   libchrome_dll.dylib                 0x000000012a0ebc40 -[BrowserWindowController showTranslateBubbleForWebContents:step:errorType:] + 816
10  libchrome_dll.dylib                 0x000000012a0ccc1d BrowserWindowCocoa::ShowTranslateBubble(content::WebContents*, translate::TranslateStep, translate::TranslateErrors::Type, bool) + 237
11  libchrome_dll.dylib                 0x000000012986601a (anonymous namespace)::ShowDefault(BrowserWindow*, content::WebContents*, translate::TranslateStep, translate::TranslateErrors::Type) + 202
12  libchrome_dll.dylib                 0x0000000129865f3e TranslateBubbleFactory::Show(BrowserWindow*, content::WebContents*, translate::TranslateStep, translate::TranslateErrors::Type) + 222
13  libchrome_dll.dylib                 0x000000011dd1c01f ChromeTranslateClient::ShowBubble(translate::TranslateStep, translate::TranslateErrors::Type) + 975
14  libchrome_dll.dylib                 0x000000011dd1bafa ChromeTranslateClient::ShowTranslateUI(translate::TranslateStep, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, translate::TranslateErrors::Type, bool) + 1594
15  libchrome_dll.dylib                 0x0000000128d601dd translate::TranslateManager::TranslatePage(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool) + 2957
16  libchrome_dll.dylib                 0x000000011e1e9474 RenderViewContextMenu::ExecTranslate() + 1124
17  libchrome_dll.dylib                 0x000000011e1e2aaa RenderViewContextMenu::ExecuteCommand(int, int) + 5850
18  libchrome_dll.dylib                 0x0000000129ed6450 RenderViewContextMenuMac::ExecuteCommand(int, int) + 1424
19  libui_base.dylib                    0x0000000112f695b6 ui::SimpleMenuModel::ActivatedAt(int, int) + 390
20  libui_base.dylib                    0x0000000112e9ff84 -[MenuControllerCocoa itemSelected:uiEventFlags:] + 1108

Maybe related to  Issue 795725 

 

Comment 1 by pbos@chromium.org, Dec 19 2017

Blocking: 795725
Let's hypothesize.

Comment 2 by tapted@chromium.org, Dec 19 2017

I haven't been able to trigger the crash under asan after commenting out the DCHECK :/

I actually have a fix already for the DCHECK -- https://chromium-review.googlesource.com/c/chromium/src/+/752783

I'll polish that up. there are probably more translate browsertests that should be running on Mac too. We do want bubbles eventually, just not until after Harmony Phase 1 (and not the "2016q2" UI since that's not launching anywhere).

Comment 3 by pbos@chromium.org, Dec 19 2017

Is the "new UX" the same thing as the 2016q2 UI and never launching? If so another way of "cleaning this up" is removing the experiment, if we're still blaming the experiment.

Also wondering what the odds are of the blocked bug actually being due to heap corruption elsewhere.

Comment 4 by tapted@chromium.org, Dec 19 2017

Nope - the "new" UX is older than the 2016q2 UX.

"enable-translate-new-ux" is now synonymous with "Use bubbles for translate on Mac rather than the infobar"

"translate-2016q2-ui" doesn't appear in chrome://flags, but it can be flipped on the command line with ./out/gn_Asan/Chromium.app/Contents/MacOS/Chromium --enable-features=TranslateUI2016Q2

#if defined(OS_MACOSX)
    {"enable-translate-new-ux", flag_descriptions::kTranslateNewUxName,
     flag_descriptions::kTranslateNewUxDescription, kOsMac,
     ENABLE_DISABLE_VALUE_TYPE(switches::kEnableTranslateNewUX,
                               switches::kDisableTranslateNewUX)},
#endif  // OS_MACOSX
#if defined(OS_LINUX) || defined(OS_WIN)
    {"translate-2016q2-ui", flag_descriptions::kTranslate2016q2UiName,
     flag_descriptions::kTranslate2016q2UiDescription,
     kOsCrOS | kOsWin | kOsLinux,
     FEATURE_VALUE_TYPE(translate::kTranslateUI2016Q2)},
#endif  // OS_LINUX || OS_WIN

Comment 5 by pbos@chromium.org, Dec 19 2017

Blocking: -795725
Unduping since it doesn't explain it as far as we know. :(
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26200f5525047de3095d426af3b6fdf4744f7e49

commit 26200f5525047de3095d426af3b6fdf4744f7e49
Author: Trent Apted <tapted@chromium.org>
Date: Wed Dec 20 00:54:05 2017

MacViews: Fix Translate Bubble Anchoring

Currently translate _bubble_ tests on Mac (not the info bar tests)
fail with a DCHECK because the translate bubble does not currently
anchor properly under MacViews. To Anchor properly the bubble needs
a parent window set before calling Widget::Show().

Fix that, and expand test coverage on Mac to include tests for the
bubble.

Note the translate bubble is not enabled by default on Mac (m65 will
continue to use infobars).

Enables on Mac:
 - AutofillInteractiveTest.AutofillAfterTranslate
 - TranslateBubbleViewBrowserTest.* (3 tests)

Bug:  781134 ,  795987 , 507442
Change-Id: I27f8489ad84eb90997ad6f319b178942a07a7a51
Reviewed-on: https://chromium-review.googlesource.com/752783
Reviewed-by: Michael Martis <martis@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525201}
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/autofill/autofill_interactive_uitest.cc
[rename] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/ui/cocoa/translate/translate_bubble_test_utils_views_cocoa.mm
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/test/BUILD.gn

Comment 7 by tapted@chromium.org, Dec 20 2017

Status: Fixed (was: Started)

Comment 8 by tapted@chromium.org, Dec 20 2017

Blocking: 795725

Sign in to add a comment