New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 712545 link

Starred by 1 user
Status: Fixed
Owner:
tapted@chromium.org
Closed: Apr 2017
Cc:
nyerramilli@chromium.org
█ msrchandra@chromium.org
█ ranjitkan@chromium.org
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.

Reported by avsha...@etouch.net, Apr 18 2017 
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

Comment 1 by avsha...@etouch.net, Apr 18 2017

Labels: hasbisect
Owner: tapted@chromium.org
Status: Assigned (was: Unconfirmed)
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.

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

Labels: ReleaseBlock-Beta
Status: Started (was: Assigned)
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.

Comment 3 by tapted@chromium.org, Apr 19 2017 

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 2017 

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

Comment 5 by pbomm...@chromium.org, Apr 21 2017 

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.

Comment 6 by tapted@chromium.org, Apr 22 2017 

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.

Comment 7 by tapted@chromium.org, Apr 22 2017

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 2017

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 2017

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

Comment 10 by tapted@chromium.org, Apr 22 2017

Status: Fixed (was: Started)

Comment 11 by brajkumar@chromium.org, Apr 25 2017

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