|
|
Created:
4 years, 5 months ago by tapted Modified:
4 years, 5 months ago Reviewers:
Robert Sesek CC:
chromium-reviews, tfarina, chrome-apps-syd-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse PostTask, not performSelector in NativeWidgetMac::Close()
In rare circumstances (more often with ASAN), a call to
[window performSelector:@selector(close) withObject:nil afterDelay:0];
doesn't get run when ViewsTestBase::TearDown() does
base::RunLoop run_loop;
run_loop.RunUntilIdle();
ViewsTestBase then destroys other stuff. And, when the posted selector
eventually gets executed, bad stuff happens.
To fix, post a "regular" task, invoking -[NSWindow close]. Use a block
to retain the window receiving the close.
BUG=624648
Committed: https://crrev.com/961644ad65fe2d43190d79e99f8b00a661026f6f
Cr-Commit-Position: refs/heads/master@{#403400}
Patch Set 1 #
Total comments: 2
Patch Set 2 : no parens #Messages
Total messages: 14 (8 generated)
Description was changed from ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff and when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close] in a block to retain the window receiving the close. BUG=624648 ========== to ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. Then, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close] in a block to retain the window receiving the close. BUG=624648 ==========
tapted@chromium.org changed reviewers: + rsesek@chromium.org
Description was changed from ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. Then, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close] in a block to retain the window receiving the close. BUG=624648 ========== to ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. Then, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close]. Use a block to retain the window receiving the close. BUG=624648 ==========
Description was changed from ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. Then, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close]. Use a block to retain the window receiving the close. BUG=624648 ========== to ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. And, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close]. Use a block to retain the window receiving the close. BUG=624648 ==========
Hi Robert, please take a look
LGTM The main reason we use the -performSelector:… approach is that a posted task can get pumped during nested run loops associated with -performClose: and mouseDown:. Since Cocoa won't be spinning any such loops, this shouldn't result in zombies or other reentrancy issues. https://codereview.chromium.org/2115453002/diff/1/ui/views/widget/native_widg... File ui/views/widget/native_widget_mac.mm (right): https://codereview.chromium.org/2115453002/diff/1/ui/views/widget/native_widg... ui/views/widget/native_widget_mac.mm:376: base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::BindBlock(^() { nit: just ^{ will work
Thanks Robert! https://codereview.chromium.org/2115453002/diff/1/ui/views/widget/native_widg... File ui/views/widget/native_widget_mac.mm (right): https://codereview.chromium.org/2115453002/diff/1/ui/views/widget/native_widg... ui/views/widget/native_widget_mac.mm:376: base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::BindBlock(^() { On 2016/07/01 02:31:38, Robert Sesek wrote: > nit: just ^{ will work Done.
The CQ bit was checked by tapted@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org Link to the patchset: https://codereview.chromium.org/2115453002/#ps20001 (title: "no parens")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. And, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close]. Use a block to retain the window receiving the close. BUG=624648 ========== to ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. And, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close]. Use a block to retain the window receiving the close. BUG=624648 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. And, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close]. Use a block to retain the window receiving the close. BUG=624648 ========== to ========== Use PostTask, not performSelector in NativeWidgetMac::Close() In rare circumstances (more often with ASAN), a call to [window performSelector:@selector(close) withObject:nil afterDelay:0]; doesn't get run when ViewsTestBase::TearDown() does base::RunLoop run_loop; run_loop.RunUntilIdle(); ViewsTestBase then destroys other stuff. And, when the posted selector eventually gets executed, bad stuff happens. To fix, post a "regular" task, invoking -[NSWindow close]. Use a block to retain the window receiving the close. BUG=624648 Committed: https://crrev.com/961644ad65fe2d43190d79e99f8b00a661026f6f Cr-Commit-Position: refs/heads/master@{#403400} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/961644ad65fe2d43190d79e99f8b00a661026f6f Cr-Commit-Position: refs/heads/master@{#403400} |