Commit 3c6cf7bf authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[base] Enable PM_QS_ALLEVENTS in ProcessPumpReplacementMessage by default

And remove the PreventMessagePumpHangs experiment. It was initially
planned to be used for more than this experiment but has since reached
its expiry date without this version showing side-effects on metrics
(as intended). I'll add it back if/when doing further MessagePump
experiments.

R=chromium-metrics-reviews@google.com, fdoray@chromium.org

Bug: 1074019
Change-Id: I5b19e80380202133afa3f494268a27e6ffd06c5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2312839
Auto-Submit: Gabriel Charette <gab@chromium.org>
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791043}
parent 3a401819
...@@ -20,17 +20,6 @@ namespace base { ...@@ -20,17 +20,6 @@ namespace base {
namespace { namespace {
// Jank analysis uncovered that Windows uses native ::PeekMessage calls as an
// opportunity to yield to other threads according to some heuristics (e.g.
// presumably when there's no input but perhaps a single WM_USER message posted
// later than another thread was readied). MessagePumpForUI doesn't intend to
// give this opportunity to the kernel when invoking ::PeekMessage however. This
// experiment attempts to regain control of the pump (behind an experiment
// because of how fragile this code is -- experiments help external contributors
// diagnose regressions, e.g. crbug.com/1078475).
const Feature kPreventMessagePumpHangs{"PreventMessagePumpHangs",
FEATURE_DISABLED_BY_DEFAULT};
enum MessageLoopProblems { enum MessageLoopProblems {
MESSAGE_POST_ERROR, MESSAGE_POST_ERROR,
COMPLETION_POST_ERROR, COMPLETION_POST_ERROR,
...@@ -552,32 +541,28 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() { ...@@ -552,32 +541,28 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() {
// that peeked replacement. Note that the re-post of kMsgHaveWork may be // that peeked replacement. Note that the re-post of kMsgHaveWork may be
// asynchronous to this thread!! // asynchronous to this thread!!
// Bump the work id since ::PeekMessage may process internal events.
state_->delegate->BeforeDoInternalWork();
// The system headers don't define this; it's equivalent to PM_QS_INPUT |
// PM_QS_PAINT | PM_QS_POSTMESSAGE. i.e., anything but QS_SENDMESSAGE. Since
// we're looking to replace our kMsgHaveWork posted message, we can ignore
// sent messages (which never compete with posted messages in the initial
// PeekMessage call).
constexpr auto PM_QS_ALLEVENTS = QS_ALLEVENTS << 16;
static_assert(
PM_QS_ALLEVENTS == (PM_QS_INPUT | PM_QS_PAINT | PM_QS_POSTMESSAGE), "");
static_assert((PM_QS_ALLEVENTS & PM_QS_SENDMESSAGE) == 0, "");
MSG msg; MSG msg;
bool have_message = false; bool have_message = false;
{ {
// Bump the work id since ::PeekMessage may process internal events.
state_->delegate->BeforeDoInternalWork();
TRACE_EVENT0("base", TRACE_EVENT0("base",
"MessagePumpForUI::ProcessPumpReplacementMessage PeekMessage"); "MessagePumpForUI::ProcessPumpReplacementMessage PeekMessage");
static const auto peek_replacement_message_modifier = // The system headers don't define PM_QS_ALLEVENTS; it's equivalent to
base::FeatureList::IsEnabled(kPreventMessagePumpHangs) ? PM_QS_ALLEVENTS // PM_QS_INPUT | PM_QS_PAINT | PM_QS_POSTMESSAGE. i.e., anything but
: 0; // QS_SENDMESSAGE.
// Since we're looking to replace our kMsgHaveWork posted message, we can
have_message = // ignore sent messages (which never compete with posted messages in the
::PeekMessage(&msg, nullptr, 0, 0, // initial PeekMessage call).
PM_REMOVE | peek_replacement_message_modifier) != FALSE; constexpr auto PM_QS_ALLEVENTS = QS_ALLEVENTS << 16;
static_assert(
PM_QS_ALLEVENTS == (PM_QS_INPUT | PM_QS_PAINT | PM_QS_POSTMESSAGE), "");
static_assert((PM_QS_ALLEVENTS & PM_QS_SENDMESSAGE) == 0, "");
have_message = ::PeekMessage(&msg, nullptr, 0, 0,
PM_REMOVE | PM_QS_ALLEVENTS) != FALSE;
} }
// Expect no message or a message different than kMsgHaveWork. // Expect no message or a message different than kMsgHaveWork.
......
...@@ -5365,21 +5365,6 @@ ...@@ -5365,21 +5365,6 @@
] ]
} }
], ],
"PreventMessagePumpHangs": [
{
"platforms": [
"windows"
],
"experiments": [
{
"name": "PreventMessagePumpHangs",
"enable_features": [
"PreventMessagePumpHangs"
]
}
]
}
],
"PreviewsLitePageRedirect": [ "PreviewsLitePageRedirect": [
{ {
"platforms": [ "platforms": [
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment