Public bug reported:
I've reviewed the behaviour of Oxide if Qt or an application executes a
nested QEventLoop on the main thread. This is needed to ensure we can
safely handle drag and drop:
- Scenario 1: QEventLoop executed with no Chromium code on the stack.
In this case, the nested loop will pump the Chromium event queue as normal, and
this is fine because there are no nested Chromium tasks.
- Scenario 2: QEventLoop executed from a non-nested Chromium task.
In this case, the nested loop will pump the Chromium event queue by re-entering
oxide::qt::MessagePump::RunOneTask()
- It doesn't look like there's any re-entrancy issues here.
- MessageLoop::DoWork() will not run any tasks because the MessageLoop is in
a task and nestable tasks haven't been explicitly allowed
(MessageLoop::nestable_tasks_allowed_ is set to false in RunTask).
- MessageLoop::DoDelayedWork() - same as DoWork()
- MessageLoop::DoIdleWork() will process tasks from the deferred non-nestable
work queue.
The last point is a bug, as DoIdleWork() should not run tasks in a
nested loop. However, it seems like an edge case - tasks are only added
to this work queue via nested calls to DoWork() and DoDelayedWork() when
nestable tasks are allowed but the task isn't nestable. We don't do this
anywhere in Oxide, although that doesn't mean it couldn't happen
elsewhere in Chromium.
- Scenario 3: QEventLoop executed from a nested Chromium task (via RunLoop::Run)
As 2, above. Nested tasks are still blocked in DoIdle and DoDelayedWork because
RunTask clears nestable_tasks_allowed_ before running the nested task. However,
DoIdleWork will behave correctly here because the RunLoop::run_depth_ check in
MessageLoop::ProcessNextDelayedNonNestableTask will work.
It seems like we should detect re-entrancy from a nested QEventLoop in
oxide::qt::MessagePump::RunOneTask(). We could do this by adding an extra bit
to RunState, that we set when calling in to MessageLoop.
- If we re-enter from a nested QEventLoop, the extra bit on the current
RunState will be set. In this case, we should create a new RunLoop instance
before calling in to MessageLoop.
- A nested RunLoop creates a new RunState in our MessagePump. In this case, we
won't trigger the re-entrancy detection (and we don't need to)
After this is fixed, a nested QEventLoop created outside of Oxide will
process Qt events but won't run any Oxide or Chromium tasks. The only
way to run a nested loop that processes nestable Oxide or Chromium tasks
is to use MessageLoop::ScopedNestableTasksAllower and RunLoop, but that
can only be done from code in Oxide.
** Affects: oxide
Importance: Medium
Status: Triaged
** Changed in: oxide
Importance: Undecided => Medium
** Changed in: oxide
Status: New => Triaged
--
You received this bug notification because you are a member of Ubuntu
WebApps bug tracking, which is subscribed to Oxide.
https://bugs.launchpad.net/bugs/1536797
Title:
Fix nested message loop handling
Status in Oxide:
Triaged
Bug description:
I've reviewed the behaviour of Oxide if Qt or an application executes
a nested QEventLoop on the main thread. This is needed to ensure we
can safely handle drag and drop:
- Scenario 1: QEventLoop executed with no Chromium code on the stack.
In this case, the nested loop will pump the Chromium event queue as normal,
and this is fine because there are no nested Chromium tasks.
- Scenario 2: QEventLoop executed from a non-nested Chromium task.
In this case, the nested loop will pump the Chromium event queue by
re-entering oxide::qt::MessagePump::RunOneTask()
- It doesn't look like there's any re-entrancy issues here.
- MessageLoop::DoWork() will not run any tasks because the MessageLoop is
in a task and nestable tasks haven't been explicitly allowed
(MessageLoop::nestable_tasks_allowed_ is set to false in RunTask).
- MessageLoop::DoDelayedWork() - same as DoWork()
- MessageLoop::DoIdleWork() will process tasks from the deferred
non-nestable work queue.
The last point is a bug, as DoIdleWork() should not run tasks in a
nested loop. However, it seems like an edge case - tasks are only
added to this work queue via nested calls to DoWork() and
DoDelayedWork() when nestable tasks are allowed but the task isn't
nestable. We don't do this anywhere in Oxide, although that doesn't
mean it couldn't happen elsewhere in Chromium.
- Scenario 3: QEventLoop executed from a nested Chromium task (via
RunLoop::Run)
As 2, above. Nested tasks are still blocked in DoIdle and DoDelayedWork
because RunTask clears nestable_tasks_allowed_ before running the nested task.
However, DoIdleWork will behave correctly here because the RunLoop::run_depth_
check in MessageLoop::ProcessNextDelayedNonNestableTask will work.
It seems like we should detect re-entrancy from a nested QEventLoop in
oxide::qt::MessagePump::RunOneTask(). We could do this by adding an extra bit
to RunState, that we set when calling in to MessageLoop.
- If we re-enter from a nested QEventLoop, the extra bit on the current
RunState will be set. In this case, we should create a new RunLoop instance
before calling in to MessageLoop.
- A nested RunLoop creates a new RunState in our MessagePump. In this case,
we won't trigger the re-entrancy detection (and we don't need to)
After this is fixed, a nested QEventLoop created outside of Oxide will
process Qt events but won't run any Oxide or Chromium tasks. The only
way to run a nested loop that processes nestable Oxide or Chromium
tasks is to use MessageLoop::ScopedNestableTasksAllower and RunLoop,
but that can only be done from code in Oxide.
To manage notifications about this bug go to:
https://bugs.launchpad.net/oxide/+bug/1536797/+subscriptions
--
Mailing list: https://launchpad.net/~ubuntu-webapps-bugs
Post to : [email protected]
Unsubscribe : https://launchpad.net/~ubuntu-webapps-bugs
More help : https://help.launchpad.net/ListHelp
