1299594 - remove dead CloneManagees and related code

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Nathan Froyd [:froydnj]]

As suggested in bug 1297804 comment 9, all this stuff is dead code, so we should get rid of it.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: part 1 - remove CloneManagees/CloneToplevel code from IPDL

CloneOpenedToplevels, which is never called, is the only interesting
caller of CloneToplevel.  And CloneToplevel, in turn, is the only
interesting caller of CloneManagees.  Which means we can ditch all this
code for a decent amount of space savings, both in code and writable
static data (no more useless virtual function entries in vtables).

This part is the big win, about 128K on x86-64 Linux.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: part 2 - remove opened actor tracking from IToplevelProtocol

The only thing we needed opened actor tracking for was the ability to
clone all the actors.  But now that we no longer have support for
cloning actors, we no longer need to track the actors that we've cloned,
which makes a number of things significantly simpler.

I'm less sure about this patch--whether or not I did it correctly--so I split
this one out as a separate part to make it hopefully easier to review.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8786902 [details] [diff] [review]
part 2 - remove opened actor tracking from IToplevelProtocol

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

I wish I could give this r++!

::: gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -732,5 @@
>  
>  bool
>  CompositorBridgeParent::Bind(Endpoint<PCompositorBridgeParent>&& aEndpoint)
>  {
> -  if (!aEndpoint.Bind(this, nullptr)) {

I think removing this is incorrect. We just want to get rid of the null parameter.

::: ipc/glue/ProtocolUtils.cpp
@@ +72,5 @@
>  }
>  
>  IToplevelProtocol::~IToplevelProtocol()
>  {
>    StaticMutexAutoLock al(gProtocolMutex);

Let's get rid of this mutex too (the var as well as the acquisition).

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: part 3 - remove gProtocolMutex

It no longer protects any global state, and is therefore pure overhead.

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: part 2 - remove opened actor tracking from IToplevelProtocol

With the removed Bind() call put back.  I was removing them before I realized
what they were used for, and clearly forgot to put that one back.  Carrying
over r+.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee21ab5cb479
part 1 - remove CloneManagees/CloneToplevel code from IPDL; r=billm
https://hg.mozilla.org/integration/mozilla-inbound/rev/67e8b862bdb0
part 2 - remove opened actor tracking from IToplevelProtocol; r=billm
https://hg.mozilla.org/integration/mozilla-inbound/rev/47f3a6275d66
part 3 - remove gProtocolMutex; r=billm

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ee21ab5cb479
https://hg.mozilla.org/mozilla-central/rev/67e8b862bdb0
https://hg.mozilla.org/mozilla-central/rev/47f3a6275d66