1400407 - Cleanup WebVR Dead Code

Status: RESOLVED FIXED

(Core :: WebVR, enhancement, P3)

Description

[:kip (Kearwood Gilbert)]

Once Bug 1381085 lands, WebVR won't be dependent on some functions that can be refactored out and deleted.

Comment 1

[:kip (Kearwood Gilbert)]

This cleanup can wait until FF58+

Comment 2

[:kip (Kearwood Gilbert)]

Note that Bug 1381085 must land first

Comment 3

[:kip (Kearwood Gilbert)]

Attachment: Bug 1400407 - Part 1: Cleanup WebVR dead code

- VRManagerChild no longer needs to be a TextureForwarder
- VRManagerParent no longer descends from HostIPCAllocator or ShmemAllocator
- PVRManager no longer manages PTexture's
- VRLayerParent::mSize was not used and has been removed

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

Comment 4

[:kip (Kearwood Gilbert)]

Attachment: Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

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

Comment 5

[:kip (Kearwood Gilbert)]

Attachment: Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

- This patch is the same as one from Bug 1382104 (Remove IsMirror concept
  in favor of checking forwarder).
- It is safe to uplift this patch without the rest of Bug 1382104 as long
  as the remaining Bug 1381084 is also uplifted.

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

Comment 6

[:kip (Kearwood Gilbert)]

Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

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

Comment 7

[:kip (Kearwood Gilbert)]

Comment on attachment 8908850 [details]
Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

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

Comment 8

[:kip (Kearwood Gilbert)]

Comment on attachment 8908851 [details]
Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

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

Comment 9

[:kip (Kearwood Gilbert)]

Comment on attachment 8908850 [details]
Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

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

Comment 10

[:kip (Kearwood Gilbert)]

Comment on attachment 8908851 [details]
Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

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

Comment 11

[:kip (Kearwood Gilbert)]

The dependency patch from Bug 1381085 has been moved to Bug 1400457.

Please do not land this until after Bug 1400457 lands.

Comment 12

[:kip (Kearwood Gilbert)]

Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

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

Comment 13

[:kip (Kearwood Gilbert)]

Comment on attachment 8908850 [details]
Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/180466/diff/3-4/

Comment 14

[:kip (Kearwood Gilbert)]

Comment on attachment 8908851 [details]
Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/180468/diff/3-4/

Comment 15

[:kip (Kearwood Gilbert)]

Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

https://reviewboard.mozilla.org/r/180464/#review190738

::: gfx/gl/SharedSurface.cpp:326
(Diff revision 3)
>      while (!mRecycleFreePool.empty()) {
>          RefPtr<layers::SharedSurfaceTextureClient> cur = mRecycleFreePool.front();
>          mRecycleFreePool.pop();
>  
>          if (cur->Surf()->mSize == size){
> -            // In the general case, textureClients transit textures through
> +            if (aLayersChannel && aLayersChannel == cur->GetAllocator()) {

@jgilbert: VRManagerChild is no longer a TextureForwarder, so I am removing the special case check for VRMAnagerChild here.

Comment 16

[:kip (Kearwood Gilbert)]

@jgilbert: For the part1 patch, could you take a look at the change to gfx/gl/SharedSurface.cpp?  See Comment 15 for some context.
@daoshengmu: Could you please review the rest of the patch?

Comment 17

[:kip (Kearwood Gilbert)]

Comment on attachment 8908850 [details]
Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

https://reviewboard.mozilla.org/r/180466/#review190742

::: commit-message-14b36:5
(Diff revision 4)
> +Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code
> +- Refactored out Canvas layer mirror specialization, no longer needed
> +  as we are no longer implementing a TextureForwarder to submit VR frames.
> +
> +MozReview-Commit-ID: ArZO6M9kNLg

I am assuming that the "mirror" functionality won't be needed in the future for things other than WebVR.  As WebVR is no longer dependent on it, I suspect you will be glad to see this [now dead code] cleaned up.  Please advise if otherwise.

Comment 18

[:kip (Kearwood Gilbert)]

Comment on attachment 8908851 [details]
Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

https://reviewboard.mozilla.org/r/180468/#review190744

::: commit-message-e070f:1
(Diff revision 4)
> +Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

This is again the same patch from Bug 1382104 - Remove IsMirror concept in favor of checking forwarders.

In this case; however, we don't have to worry about the "checking forwarders" part, as the mirroring code will not be hit at all any more.

Comment 19

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8908850 [details]
Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

https://reviewboard.mozilla.org/r/180466/#review190838

Comment 20

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8908851 [details]
Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

https://reviewboard.mozilla.org/r/180468/#review190840

Comment 21

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

https://reviewboard.mozilla.org/r/180464/#review190844

Comment 22

[Kelsey Gilbert [:jgilbert]]

(In reply to :kip (Kearwood Gilbert) from comment #15)
> Comment on attachment 8908849 [details]
> Bug 1400407 - Part 1: Cleanup WebVR dead code
> 
> https://reviewboard.mozilla.org/r/180464/#review190738
> 
> ::: gfx/gl/SharedSurface.cpp:326
> (Diff revision 3)
> >      while (!mRecycleFreePool.empty()) {
> >          RefPtr<layers::SharedSurfaceTextureClient> cur = mRecycleFreePool.front();
> >          mRecycleFreePool.pop();
> >  
> >          if (cur->Surf()->mSize == size){
> > -            // In the general case, textureClients transit textures through
> > +            if (aLayersChannel && aLayersChannel == cur->GetAllocator()) {
> 
> @jgilbert: VRManagerChild is no longer a TextureForwarder, so I am removing
> the special case check for VRMAnagerChild here.

This change is fine.

Comment 23

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

https://reviewboard.mozilla.org/r/180464/#review190888

LGTM.

Comment 24

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 70bc869c7e58 -d b0c2046983c6: rebasing 423904:70bc869c7e58 "Bug 1400407 - Part 1: Cleanup WebVR dead code r=daoshengmu,jgilbert"
merging gfx/vr/ipc/VRLayerParent.h
rebasing 423905:7fb46e4d60a5 "Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code r=jgilbert"
merging dom/canvas/CanvasRenderingContext2D.cpp
merging dom/canvas/WebGLContext.h
rebasing 423906:dabd62b62a07 "Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR r=jgilbert" (tip)
merging dom/canvas/WebGLContext.cpp
merging dom/canvas/WebGLContext.h
warning: conflicts while merging dom/canvas/WebGLContext.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6daba05eb240
Part 1: Cleanup WebVR dead code,r=daoshengmu,r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/53c7ef6743d9
Part 2: Cleanup DOM Canvas mirroring dead code,r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/83203c2de90a
Part 3: Remove IsMirror concept, as it is no longer used by WebVR,r=jgilbert

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6daba05eb240
https://hg.mozilla.org/mozilla-central/rev/53c7ef6743d9
https://hg.mozilla.org/mozilla-central/rev/83203c2de90a