New issue
Advanced search Search tips

Issue 787551 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 783774



Sign in to add a comment

Deferred quit broken for MessagePumpNSRunLoop and MessagePumpNSApplication

Project Member Reported by mastiz@chromium.org, Nov 21 2017

Issue description

(second attempt after  crbug.com/785165 , might still be off but I think I'm getting closer)

While debugging some flaky Mac browser tests I came across this weird behavior where certain nested runloop never do any work.

It seems like, for MessagePumpNSRunLoop and MessagePumpNSApplication, the case where keep_running_ is initially false in DoRun() is not handled well.

MessageLoop already handles the case where Quit() has been called before Run(), so reaching DoRun() must mean that work must start.

AFAIK the bug only reproduces in the convoluted scenario of deferred quits that involve yet another nested loop being started exactly between the quit request and its deferred execution.
 

Comment 1 by mastiz@chromium.org, Nov 22 2017

Blocking: 783774
Components: Internals>Core
Status: Assigned (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 5 2017

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

commit cfa89ca5781dbe9b17125415fae0d0420191b9c5
Author: Mikel Astiz <mastiz@chromium.org>
Date: Tue Dec 05 16:12:33 2017

Fix mac pumps potentially doing nothing in DoRun()

Prior to this patch, DoRun() in MessagePumpNSRunLoop and
MessagePumpNSApplication didn't handle well the case where keep_running_
is initially false.

This situation reproduces in the convoluted scenario of deferred quits
that involve yet another nested loop being started exactly between the
quit request and its deferred execution, which is actually exercised in
some browser tests.

The proposed solution mimics what other analogous pumps do.

Bug:  787551 
Change-Id: Ida8c93f9bc189477e5ad16f86e88ac1613160ba2
Reviewed-on: https://chromium-review.googlesource.com/782563
Reviewed-by: Mike Pinkerton <pinkerton@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521709}
[modify] https://crrev.com/cfa89ca5781dbe9b17125415fae0d0420191b9c5/base/message_loop/message_pump_mac.mm
[modify] https://crrev.com/cfa89ca5781dbe9b17125415fae0d0420191b9c5/ui/base/cocoa/constrained_window/constrained_window_animation_unittest.mm

Status: Fixed (was: Assigned)

Sign in to add a comment