1449620 - Introduce the notion of an APZ updater thread

Status: RESOLVED FIXED

(Core :: Panning and Zooming, enhancement)

Description

[Kartikaya Gupta (email:[e-mail])]

In bug 1441916 we added an APZ sampler thread. But we need to split that so the "sampler" thread only has functions that are sampling-related, while a new "updater" thread does the updating bits. This is necessary to complete bug 1391318 because we'll want the updating stuff to happen on the scene builder thread while the sampling happens on the render backend thread.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=927b75f2df2c8dc3d34e6b2e66e23def7a9726d9

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1449620 - Extract an APZUpdater class from APZSampler.

The APZUpdater class holds the methods that are to be run on the updater
thread. Note that there are a few differences between the APZSampler and
APZUpdater classes - most notably, APZSampler no longer has a
"RunOnSamplerThread" function because there should never be any need to
dispatch runnables to the sampler thread. There is still a
RunOnUpdaterThread in APZUpdater, as well as a mechanism for dispatching
runnables to the controller thread via the updater thread.

Review commit: https://reviewboard.mozilla.org/r/232244/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/232244/

Comment 3

[Botond Ballo [:botond]]

Comment on attachment 8963345 [details]
Bug 1449620 - Extract an APZUpdater class from APZSampler.

https://reviewboard.mozilla.org/r/232244/#review237712

::: gfx/layers/apz/public/APZUpdater.h:10
(Diff revision 1)
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef mozilla_layers_APZUpdater_h
> +#define mozilla_layers_APZUpdater_h
> +
> +#include "base/message_loop.h"

Does anything actually require this include? If it's just Runnable, nsThreadUtils.h would be a more targeted include.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

(In reply to Botond Ballo [:botond] from comment #3)
> Does anything actually require this include? If it's just Runnable,
> nsThreadUtils.h would be a more targeted include.

Yeah, it was just Runnable. Updated include to nsThreadUtils.h, that does seem a lot better, thanks! I also trimmed some of the forward-declarations that aren't needed any more - there were some that were only needed in one of {APZSampler, APZUpdater}.h and I had just copied the file so they were in both.

Finally I also added an APZUpdater alongside the APZSampler in a couple of the gtest files to make those pass (the try push above shows gtest failures because I missed this).

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8963345 [details]
Bug 1449620 - Extract an APZUpdater class from APZSampler.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232244/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b4ca22e460c
Extract an APZUpdater class from APZSampler. r=botond

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/2b4ca22e460c