Move APZ code into gfx/layers/apz

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
mozilla31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

I'd like to move the following files into gfx/layers/apz:

 gfx/layers/composite/APZCTreeManager.{cpp,h}
 gfx/layers/ipc/AsyncPanZoomController.{cpp,h}
 gfx/layers/ipc/GestureEventListener.{cpp,h}
 gfx/layers/ipc/Axis.{cpp,h}
 gfx/layers/ipc/GeckoContentController.h

and maybe also these:

 widget/xpwidgets/APZCCallbackHelper.{cpp,h}
 widget/xpwidgets/ActiveElementManager.{cpp,h} (coming in bug 976605)

I'm undecided as of yet whether or not we want to have a directory structure within gfx/layers/apz. It might make sense to keep the widget pieces separate from the rest (and there will probably be more widget pieces as time goes on).

Additionally some of the structs shoved into FrameMetrics.h (ScrollableLayerGuid and ZoomConstraints) should be moved into gfx/layers/apz as well.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #0)
> I'm undecided as of yet whether or not we want to have a directory structure
> within gfx/layers/apz. It might make sense to keep the widget pieces
> separate from the rest (and there will probably be more widget pieces as
> time goes on).

I think it makes sense to separate the classes on the parent side (APZC[TM], GEL) from the classes on the child side (APZCCallbackHelper and such) into different directories.
This moves files but breaks the build. (Split patch for easier review, but will land them squashed together)
Assignee: nobody → bugmail.mozilla
Attachment #8406866 - Flags: review?(jmuizelaar)
Attachment #8406866 - Flags: review?(botond)
Posted patch Part 2 - Fix up includes (obsolete) — Splinter Review
Attachment #8406867 - Flags: review?(jmuizelaar)
Attachment #8406867 - Flags: review?(botond)
Comment on attachment 8406866 [details] [diff] [review]
Part 1 - Move files into gfx/layers/apz

Review of attachment 8406866 [details] [diff] [review]:
-----------------------------------------------------------------

This seems fine in principle.

::: gfx/layers/moz.build
@@ +102,5 @@
> +    'apz/src/AsyncPanZoomController.h',
> +    'apz/src/Axis.h',
> +    'apz/src/GestureEventListener.h',
> +    'apz/src/TaskThrottler.h',
> +    'apz/util/APZCCallbackHelper.h',

Does all of this stuff need to be exported?
Attachment #8406866 - Flags: review?(jmuizelaar) → review+
Attachment #8406867 - Flags: review?(jmuizelaar) → review+
(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> Comment on attachment 8406866 [details] [diff] [review]
> Part 1 - Move files into gfx/layers/apz
> 
> Review of attachment 8406866 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems fine in principle.
> 
> ::: gfx/layers/moz.build
> @@ +102,5 @@
> > +    'apz/src/AsyncPanZoomController.h',
> > +    'apz/src/Axis.h',
> > +    'apz/src/GestureEventListener.h',
> > +    'apz/src/TaskThrottler.h',
> > +    'apz/util/APZCCallbackHelper.h',
> 
> Does all of this stuff need to be exported?

At the moment, yes, because the exported AsyncPanZoomController.h includes them, and there is code that includes that exported AsyncPanZoomController.h. I can probably do some follow-up work so that we can encapsulate more of this code and not export it.
Unfortunately TBPL is not too happy with my changes. Will need to investigate.

https://tbpl.mozilla.org/?tree=Try&rev=66eedd2dcd58
Comment on attachment 8406866 [details] [diff] [review]
Part 1 - Move files into gfx/layers/apz

Review of attachment 8406866 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/moz.build
@@ +101,5 @@
> +    'apz/src/APZCTreeManager.h',
> +    'apz/src/AsyncPanZoomController.h',
> +    'apz/src/Axis.h',
> +    'apz/src/GestureEventListener.h',
> +    'apz/src/TaskThrottler.h',

Please add a comment saying that the exporting of things from 'src' is temporary until we extract an interface from APZCTreeManager and put it into 'public' (as discussed on IRC).
Attachment #8406866 - Flags: review?(botond) → review+
Comment on attachment 8406866 [details] [diff] [review]
Part 1 - Move files into gfx/layers/apz

Review of attachment 8406866 [details] [diff] [review]:
-----------------------------------------------------------------

Also you'll probably want to move ActiveElementManager.{h,cpp}, which has just landed (on m-i), as well.
Attachment #8406867 - Flags: review?(botond) → review+
Also updated to move widget/xpwidget/ActiveElementManager, carrying r+ since fundamentally nothing has changed.
Attachment #8406866 - Attachment is obsolete: true
Attachment #8407740 - Flags: review+
Updated to include some comments in the moz.build file, also updating namespaces of the things moved over from widget/xpwidget, and fixing various build errors.

Try push at https://tbpl.mozilla.org/?tree=Try&rev=4f4e1fb087de and I'll squash these two parts together for landing.
Attachment #8406867 - Attachment is obsolete: true
Attachment #8407741 - Flags: review?(botond)
Comment on attachment 8407741 [details] [diff] [review]
Part 2 - Fix up includes and namespaces (v2)

Review of attachment 8407741 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gonk/ParentProcessController.cpp
@@ +25,5 @@
>      NS_IMETHOD Run() {
>          MOZ_ASSERT(NS_IsMainThread());
>          nsCOMPtr<nsIContent> content = nsLayoutUtils::FindContentFor(mFrameMetrics.GetScrollId());
>          if (content) {
> +            mozilla::layers::APZCCallbackHelper::UpdateSubFrame(content, mFrameMetrics);

layers:: would be sufficient (I don't care much though)
Attachment #8407741 - Flags: review?(botond) → review+
New try push with more unified build fixes (see latest two dep bugs added):

https://tbpl.mozilla.org/?tree=Try&rev=93783f4703b1
https://hg.mozilla.org/mozilla-central/rev/f9963d292e66
https://hg.mozilla.org/mozilla-central/rev/953430fb3c69
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.