Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Issue 1163423006: Revert of Don't peek messages in the MessagePumpForUI class when we receive our kMsgHaveWork message. (Closed)

Created:
5 years, 6 months ago by kjellander_chromium
Modified:
5 years, 6 months ago
CC:
chromium-reviews, erikwright+watch_chromium.org, sadrul
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Don't peek messages in the MessagePumpForUI class when we receive our kMsgHaveWork message. (patchset #13 id:240001 of https://codereview.chromium.org/1156503005/) Reason for revert: I suspect this CL breaks Windows 8 content_unittests, as it's listed both in https://build.chromium.org/p/chromium.fyi/builders/Win8%20Tests%20%281%29/builds/8675 and our WebRTC-specific waterfall: https://build.chromium.org/p/chromium.webrtc/builders/Win8%20Tester/builds/14164 I'll try to reland if it turns out not to solve the breakage. Original issue's description: > Don't peek messages in the MessagePumpForUI class when we receive our kMsgHaveWork message. > > Currently the MessagePumpForUI class peeks Windows messages when we receive the kMsgHaveWork message in > our main message loop and in nested modal loops. This is because the posted message starves the message loop > of input and other lower priority messages. While this is ok for the main message loop our peeking and dispatching > messages in nested loops is wrong and violates the silent contract where in the nested loop should be the one peeking > and dispatching messages. > > To fix this the approach we are taking is to create a worker thread which uses a waitable timer of 3 ms which posts > the kMsgHaveWork message to the main loop when the timer fires. As a result we can safely delete all the code > in the MessagePumpForUI::ScheduleWork function and simplify the ProcessPumpReplacementMessage function. > > The MessagePumpForUI::ScheduleWork function now checks the delay for the task at the head of the queue > If it is 0 then we post the message right away as it is a regular task. Added functions (GetNewlyAddedTaskDelay) to the MessagePump::Delegate class and the IncomingTaskQueue for this. > > The other change here is to change the GPU process to use the IO message pump by default and use the UI pump only > if we are using opengl. Reason being this patch causes a delay in processing tasks due to the worker thread which > causes tests like webgl_conformance to fail. We will continue working on addressing this over the coming days. > > BUG=492837 > R=cpu > > Committed: https://crrev.com/b8e126c8b532b1327f38afe2bdf59aa5ff933971 > Cr-Commit-Position: refs/heads/master@{#333572} TBR=cpu@chromium.org,jar@chromium.org,kbr@chromium.org,geofflang@chromium.org,scottmg@chromium.org,ananta@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=492837 Committed: https://crrev.com/e799035ef225f9238d5aa7bea125d6d0289db3cd Cr-Commit-Position: refs/heads/master@{#333700}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -147 lines) Patch
M base/message_loop/incoming_task_queue.h View 1 chunk +0 lines, -3 lines 0 comments Download
M base/message_loop/incoming_task_queue.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M base/message_loop/message_loop.h View 1 chunk +0 lines, -1 line 0 comments Download
M base/message_loop/message_loop.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M base/message_loop/message_pump.h View 1 chunk +0 lines, -3 lines 0 comments Download
M base/message_loop/message_pump_win.h View 3 chunks +0 lines, -31 lines 0 comments Download
M base/message_loop/message_pump_win.cc View 4 chunks +58 lines, -86 lines 0 comments Download
M content/gpu/gpu_main.cc View 1 chunk +1 line, -14 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
kjellander_chromium
5 years, 6 months ago (2015-06-10 07:53:25 UTC) #1
Created Revert of Don't peek messages in the MessagePumpForUI class when we
receive our kMsgHaveWork message.

Powered by Google App Engine
This is Rietveld 408576698