1345564 - Remove PVRManager::GetImmediateSensorState sync IPC call and related code

Status: RESOLVED FIXED

(Core :: WebVR, enhancement)

Description

[:kip (Kearwood Gilbert)]

Due to earlier refactoring and changes in the WebVR API, we can remove this code and it's related sync IPC call.

Comment 1

[:kip (Kearwood Gilbert)]

Attachment: Bug 1345564 - Reduce sync IPC calls used by WebVR

- Eliminated the VRDisplay.GetImmediateSensorState sync call
  and associated code as it is no longer needed.

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

Comment 2

[(no longer active)]

Comment on attachment 8845042 [details]
Bug 1345564 - Reduce sync IPC calls used by WebVR

https://reviewboard.mozilla.org/r/118268/#review120128

::: gfx/vr/ipc/PVRManager.ipdl
(Diff revision 1)
>    // Reset the sensor of the display identified by aDisplayID so that the current
>    // sensor state is the "Zero" position.
>    async ResetSensor(uint32_t aDisplayID);
>  
>    sync GetSensorState(uint32_t aDisplayID) returns(VRHMDSensorState aState);
> -  sync GetImmediateSensorState(uint32_t aDisplayID) returns(VRHMDSensorState aState);

You need to remove the corresponding entry from sync-messages.ini, and get an IPC peer to sign off on it.  (Note that after bug 1345111, the build will fail if you forget to do that.)

Comment 3

[:kip (Kearwood Gilbert)]

Comment on attachment 8845042 [details]
Bug 1345564 - Reduce sync IPC calls used by WebVR

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

Comment 4

[:kip (Kearwood Gilbert)]

(In reply to :Ehsan Akhgari from comment #2)
> Comment on attachment 8845042 [details]
> Bug 1345564 - Reduce sync IPC calls used by WebVR
> 
> https://reviewboard.mozilla.org/r/118268/#review120128
> 
> ::: gfx/vr/ipc/PVRManager.ipdl
> (Diff revision 1)
> >    // Reset the sensor of the display identified by aDisplayID so that the current
> >    // sensor state is the "Zero" position.
> >    async ResetSensor(uint32_t aDisplayID);
> >  
> >    sync GetSensorState(uint32_t aDisplayID) returns(VRHMDSensorState aState);
> > -  sync GetImmediateSensorState(uint32_t aDisplayID) returns(VRHMDSensorState aState);
> 
> You need to remove the corresponding entry from sync-messages.ini, and get
> an IPC peer to sign off on it.  (Note that after bug 1345111, the build will
> fail if you forget to do that.)

Thanks for spotting that!  I'll also be sure to get peer review.

Comment 5

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8845042 [details]
Bug 1345564 - Reduce sync IPC calls used by WebVR

https://reviewboard.mozilla.org/r/118268/#review120240

LGTM

Comment 6

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 8845042 [details]
Bug 1345564 - Reduce sync IPC calls used by WebVR

https://reviewboard.mozilla.org/r/118268/#review120434

Comment 7

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 15 changes to 15 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: 
remote: sync-messages.ini altered in changeset 70ef581fde5c without IPC peer review
remote: 
remote: 
remote: Changes to sync-messages.ini in this repo require review from a IPC peer in the form of r=...
remote: This is to ensure that we behave responsibly by not adding sync IPC messages that cause performance issues needlessly. We appreciate your understanding..
remote: 
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.d_webidl hook failed
abort: push failed on remote

Comment 8

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Mozilla Autoland from comment #7)
> remote: sync-messages.ini altered in changeset 70ef581fde5c without IPC peer
> review

I opened bug 1345803 to update the hook. You should be able to land again next week.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8de58d33e08
Reduce sync IPC calls used by WebVR r=daoshengmu,Ehsan,kanru

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d8de58d33e08