Allow initializing APZ when we have a RemoteCompositorSession

RESOLVED FIXED in Firefox 51

Status

()

Core
Graphics
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: rhunt, Assigned: rhunt)

Tracking

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
Hook up creation of APZ protocols when we have a RemoteCompositorSession.
(Assignee)

Comment 1

a year ago
Created attachment 8785197 [details] [diff] [review]
hookup-rcp.patch

Enable the creation of APZ in RemoteCompositorSession.
Attachment #8785197 - Flags: review?(dvander)
Comment on attachment 8785197 [details] [diff] [review]
hookup-rcp.patch

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

Looks good, r=me with nits picked.

::: gfx/ipc/GPUProcessManager.cpp
@@ +394,5 @@
>    if (!child->SendInitialize(aRootLayerTreeId)) {
>      return nullptr;
>    }
>  
> +  APZCTreeManagerChild* apz = nullptr;

Use RefPtr here instead, and then you can remove the AddRef below.

::: gfx/ipc/RemoteCompositorSession.cpp
@@ +37,4 @@
>  }
>  
>  already_AddRefed<IAPZCTreeManager>
>  RemoteCompositorSession::GetAPZCTreeManager() const

It's cleaner to just change the return type to RefPtr<IAPZCTreeManager>. already_AddRefed is not buying us anything here.

::: gfx/layers/ipc/CompositorBridgeParent.cpp
@@ +1357,5 @@
>  PAPZCTreeManagerParent*
>  CompositorBridgeParent::AllocPAPZCTreeManagerParent(const uint64_t& aLayersId)
>  {
> +  // The main process should only be using the mRootLayerTreeID.
> +  MOZ_ASSERT(aLayersId == mRootLayerTreeID);

We usually pass 0 as the layers id, for window messages that are shared with CrossProcessCompositorBridge.

@@ +1383,5 @@
>  PAPZParent*
>  CompositorBridgeParent::AllocPAPZParent(const uint64_t& aLayersId)
>  {
> +  // The main process should only be using the mRootLayerTreeID.
> +  MOZ_ASSERT(aLayersId == mRootLayerTreeID);

Ditto here.

@@ +1410,5 @@
>  
>  bool
>  CompositorBridgeParent::RecvAsyncPanZoomEnabled(const uint64_t& aLayersId, bool* aHasAPZ)
>  {
> +  MOZ_ASSERT(aLayersId == mRootLayerTreeID);

Ditto.
Attachment #8785197 - Flags: review?(dvander) → review+
(Assignee)

Comment 3

a year ago
Created attachment 8786003 [details] [diff] [review]
hookup-rcs.patch

Fixes for the nits.
Attachment #8785197 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Keywords: checkin-needed
Summary: Hook up remote APZ → Allow initializing APZ when we have a RemoteCompositorSession

Comment 4

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/81de0e995050
Hook up creation of OOP APZ for main process. r=dvander
Keywords: checkin-needed

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/81de0e995050
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.