Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: Apr 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment
Regression : Chrome opens innumerous tabs (of same page) on clicking 'Learn more' link.
Project Member Reported by avsha...@etouch.net, Apr 18 Back to list
Chrome Version : 59.0.3071.9 (Official Build) 2ae1b9cb3ac16142bb504b4529323d14d0eb6f93-refs/branch-heads/3071@{#28} 32/64 bit
OS : Windows (7,8,10)

Test URL : https://chrome.google.com/webstore/detail/momentum/laookkfknpbbblfpciffpaejjkokdgca?utm_source=chrome-ntp-icon

What steps will reproduce the problem?
1. Launch chrome and navigate to above test URL.
2. Add an 'Momentum' extension (after installing an extension, NTP opens with a 'dialog' in it's top right corner).
3. Click on "Learn more' link in dialog and observe.

Actual : Chrome opens innumerous tabs (of same page) on clicking 'Learn more' link.

Expected : Instead, chrome should open only one tab after clicking on 'Learn more' link.

This is a regression issue broken in ‘M-59’, below is the Manual Regression range and will soon update other info.
Good build : 59.0.3050.0
Bad build : 59.0.3051.3

Note : 
1. This is Windows OS specific issue and it is not reproducible on Mac and Linux OS.
2. Above issue is seen for all the extensions that change NTP settings (ex. 'My Cats New Tab' extension)
 
Actual_Result.mp4
2.2 MB View Download
Expected_Result.mp4
1.7 MB View Download
Labels: hasbisect
Owner: tapted@chromium.org
Status: Assigned
Narrow Bisect info : 
https://chromium.googlesource.com/chromium/src/+log/bf1ac35ce1af82f04ec234402b81096ac3a0dae1..b874c08258d7ffb293ecf1f779cdf2c61d03e23f?pretty=fuller&n=10000

Suspecting: r459382 from Narrow Bisect 

@tapted : Could you please look into the issue and if possible please help to assign it to concern owner, thank you.
Labels: ReleaseBlock-Beta
Status: Started
https://chrome.google.com/webstore/detail/empty-new-tab-page/dpjamkmjmigaoobjbekmfgabipmfilij is a good extension for testing this

In debug builds, a DCHECK is hit:

[8012:12308:0419/110147.893:FATAL:extension_message_bubble_controller.cc(239)] Check failed: ACTION_BOUNDARY == user_action_ (4 vs. 0)
Backtrace:
        base::debug::StackTrace::StackTrace [0x00000001800D1AC5+69]
        base::debug::StackTrace::StackTrace [0x00000001800D15E8+24]
        logging::LogMessage::~LogMessage [0x0000000180141D16+70]
        extensions::ExtensionMessageBubbleController::OnLinkClicked [0x0000000146502F28+200]
        ExtensionMessageBubbleBridge::OnBubbleClosed [0x0000000147B1E6AB+219]
        ToolbarActionsBarBubbleViews::Close [0x00000001479BC88C+60]
        views::DialogClientView::CanClose [0x00000000056E50D8+56]
        views::NonClientView::CanClose [0x00000000056EA45A+42]
        views::Widget::Close [0x00000000056D054F+79]
        views::BubbleDialogDelegateView::OnWidgetActivationChanged [0x0000000005552959+169]
        views::Widget::OnNativeWidgetActivationChanged [0x00000000056D434D+189]
        views::DesktopNativeWidgetAura::HandleActivationChanged [0x0000000005772719+57]
        views::DesktopWindowTreeHostWin::HandleActivationChanged [0x00000000057935A8+88]
        views::HWNDMessageHandler::PostProcessActivateMessage [0x0000000005708A5F+319]
        views::HWNDMessageHandler::OnWndProc [0x00000000057085F0+672]

Oddly, this doesn't repro for other kinds of "learn more" links in these bubble types -- so far in my testing just the NTP override bubble.
Exploring stuff in https://codereview.chromium.org/2825243002/

Here's a bigger stack - there's interesting things further up.

[17300:18992:0419/140119.490:FATAL:extension_message_bubble_controller.cc(239)] Check failed: ACTION_BOUNDARY == user_action_ (4 vs. 0)
Backtrace:
        base::debug::StackTrace::StackTrace [0x0000000000C11AC5+69]
        base::debug::StackTrace::StackTrace [0x0000000000C115E8+24]
        logging::LogMessage::~LogMessage [0x0000000000C81D16+70]
**      extensions::ExtensionMessageBubbleController::OnLinkClicked [0x00000001465034B8+200]
        ExtensionMessageBubbleBridge::OnBubbleClosed [0x0000000147B1EC3B+219]
        ToolbarActionsBarBubbleViews::Close [0x00000001479BCE1C+60]
        views::DialogClientView::CanClose [0x00000000052F50D8+56]
        views::NonClientView::CanClose [0x00000000052FA45A+42]
**      views::Widget::Close [0x00000000052E054F+79]
        views::BubbleDialogDelegateView::OnWidgetActivationChanged [0x0000000005162959+169]
        views::Widget::OnNativeWidgetActivationChanged [0x00000000052E434D+189]
        views::DesktopNativeWidgetAura::HandleActivationChanged [0x0000000005382719+57]
        views::DesktopWindowTreeHostWin::HandleActivationChanged [0x00000000053A35A8+88]
        views::HWNDMessageHandler::PostProcessActivateMessage [0x0000000005318A5F+319]
        views::HWNDMessageHandler::OnWndProc [0x00000000053185F0+672]
        gfx::WindowImpl::WndProc [0x000000000607AFBC+348]
        base::win::WrappedWindowProc<&gfx::WindowImpl::WndProc> [0x0000000006078488+56]
        DispatchMessageW [0x00007FFD8D691169+1673]
        DispatchMessageW [0x00007FFD8D690EE2+1026]
        GetMenuItemInfoW [0x00007FFD8D6A0BEE+1902]
        KiUserCallbackDispatcher [0x00007FFD8E548B94+36]
        SetWindowPos [0x00007FFD8D6B2594+20]
        views::HWNDMessageHandler::Activate [0x000000000530E467+103]
        views::DesktopWindowTreeHostWin::Activate [0x00000000053A1E24+36]
        views::DesktopNativeWidgetAura::Activate [0x0000000005381593+51]
        views::Widget::Activate [0x00000000052DFAE4+36]
        BrowserView::Show [0x00000001478D80B3+67]
        content::Details<content::WebContents>::Details<content::WebContents> [0x00000001475E6356+326]
        chrome::Navigate [0x00000001475E8BF3+2851]
        Browser::OpenURLFromTab [0x0000000141B3BEE2+706]
        Browser::OpenURL [0x0000000141B3BC06+54]
**      extensions::ExtensionMessageBubbleController::OnLinkClicked [0x00000001465035BC+460]
**      ExtensionMessageBubbleBridge::OnBubbleClosed [0x0000000147B1EC3B+219]
        ToolbarActionsBarBubbleViews::Close [0x00000001479BCE1C+60]
        views::DialogClientView::CanClose [0x00000000052F50D8+56]
        views::NonClientView::CanClose [0x00000000052FA45A+42]
        views::Widget::Close [0x00000000052E054F+79]
**      ToolbarActionsBarBubbleViews::LinkClicked [0x00000001479BE001+97]
        views::Link::OnMouseReleased [0x00000000051A555B+235]
        views::View::ProcessMouseReleased [0x00000000052BFF9F+191]
        views::View::OnMouseEvent [0x00000000052BDF05+261]
        ui::EventHandler::OnEvent [0x000000000E7B2873+131]
Project Member Comment 4 by bugdroid1@chromium.org, Apr 21
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d7340b19e7dd13c8379174ad50ce2c8220637aa7

commit d7340b19e7dd13c8379174ad50ce2c8220637aa7
Author: tapted <tapted@chromium.org>
Date: Fri Apr 21 02:08:16 2017

Cater for the "double close" when clicking "Learn More" on auto-dismiss extension bubbles.

This fixes a regression from r459382 where clicking a "Learn More" link
on the NTP override bubble can spew out tabs. The problem is that
opening a tab may defocus the bubble, triggering a Close(). The
extension bubble client must still call close since there's no guarantee
that the defocus may trigger a close.

To fix, Close() needs more awareness of whether it is closed via being
deactivated by the user, via being deactivated by the delegate opening a
link, or via being explicitly closed by LinkClicked(). Currently it only
caters for two of these cases. We can more clearly capture the use case
simply by ensuring the delegate is not notified multiple times by
setting a flag.

BUG= 712545 

Review-Url: https://codereview.chromium.org/2827603005
Cr-Commit-Position: refs/heads/master@{#466228}

[modify] https://crrev.com/d7340b19e7dd13c8379174ad50ce2c8220637aa7/chrome/browser/ui/extensions/extension_message_bubble_browsertest.cc
[modify] https://crrev.com/d7340b19e7dd13c8379174ad50ce2c8220637aa7/chrome/browser/ui/extensions/extension_message_bubble_browsertest.h
[modify] https://crrev.com/d7340b19e7dd13c8379174ad50ce2c8220637aa7/chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc
[modify] https://crrev.com/d7340b19e7dd13c8379174ad50ce2c8220637aa7/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
[modify] https://crrev.com/d7340b19e7dd13c8379174ad50ce2c8220637aa7/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h

tapted@ can you please merge request the Cl to M59 branch i.e, 3071 on or before e.o.d Monday. Since we are pretty close to M59 beta launch.
r466228 isn't in Canary yet (466199). https://codereview.chromium.org/2830163003/ is a merge CL. I'll request once I've had a chance to test.
Labels: Merge-Request-59
Verified fixed in Version 60.0.3078.0 (Official Build) canary (32-bit). Requesting merge.
Project Member Comment 8 by sheriffbot@chromium.org, Apr 22
Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 9 by bugdroid1@chromium.org, Apr 22
Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9581bd5192d8ceef1d6d6a02652c990998cfbfd1

commit 9581bd5192d8ceef1d6d6a02652c990998cfbfd1
Author: tapted <tapted@chromium.org>
Date: Sat Apr 22 10:30:35 2017

[merge-m59] Cater for the "double close" when clicking "Learn More" on auto-dismiss extension bubbles.

This fixes a regression from r459382 where clicking a "Learn More" link
on the NTP override bubble can spew out tabs. The problem is that
opening a tab may defocus the bubble, triggering a Close(). The
extension bubble client must still call close since there's no guarantee
that the defocus may trigger a close.

To fix, Close() needs more awareness of whether it is closed via being
deactivated by the user, via being deactivated by the delegate opening a
link, or via being explicitly closed by LinkClicked(). Currently it only
caters for two of these cases. We can more clearly capture the use case
simply by ensuring the delegate is not notified multiple times by
setting a flag.

BUG= 712545 
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=tapted@chromium.org

Review-Url: https://codereview.chromium.org/2827603005
Cr-Commit-Position: refs/heads/master@{#466228}
(cherry picked from commit d7340b19e7dd13c8379174ad50ce2c8220637aa7)

Review-Url: https://codereview.chromium.org/2830163003
Cr-Commit-Position: refs/branch-heads/3071@{#144}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/9581bd5192d8ceef1d6d6a02652c990998cfbfd1/chrome/browser/ui/extensions/extension_message_bubble_browsertest.cc
[modify] https://crrev.com/9581bd5192d8ceef1d6d6a02652c990998cfbfd1/chrome/browser/ui/extensions/extension_message_bubble_browsertest.h
[modify] https://crrev.com/9581bd5192d8ceef1d6d6a02652c990998cfbfd1/chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc
[modify] https://crrev.com/9581bd5192d8ceef1d6d6a02652c990998cfbfd1/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
[modify] https://crrev.com/9581bd5192d8ceef1d6d6a02652c990998cfbfd1/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h

Status: Fixed
Labels: TE-Verified-M59 TE-Verified-59.0.3071.25
Verified this issue on Windows-10 using chrome latest Dev #59.0.3071.25 by following steps mentioned in the original comment, Observed chrome open's only one tab while clicking learn more link. Hence adding TE-Verified label.


712545.mp4
4.5 MB View Download
Sign in to add a comment