Closed
Bug 997699
Opened 10 years ago
Closed 10 years ago
Move CompsoitableChild/Parent to CompositableClient/Host.cpp to not expose them.
Categories
(Core :: Graphics: Layers, enhancement)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: nical, Assigned: nical)
Details
Attachments
(3 files)
14.91 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
22.00 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
6.80 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
Just like we do with TextureChild/Parent.
Comment 1•10 years ago
|
||
+1 ! And by the way, TextureClient.h should not #include "mozilla/layers/PTextureChild.h"
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8409335 -
Flags: review?(bjacob)
Assignee | ||
Comment 3•10 years ago
|
||
I had to move some of the members of CompositableParent into CompositableHost so that outside code don't chat directly with the ipdl actor anymore (which is trivial since there's a 1-1 relationship between the two) I also found and removed unused members in the process.
Attachment #8409338 -
Flags: review?(bjacob)
Comment 4•10 years ago
|
||
Comment on attachment 8409335 [details] [diff] [review] 1) Move CompositableChild to the .cpp Review of attachment 8409335 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/client/ClientLayerManager.cpp @@ +12,5 @@ > #include "mozilla/Hal.h" > #include "mozilla/dom/ScreenOrientation.h" // for ScreenOrientation > #include "mozilla/dom/TabChild.h" // for TabChild > #include "mozilla/hal_sandbox/PHal.h" // for ScreenConfiguration > +#include "mozilla/layers/CompositableClient.h" // for CompositableClient, etc Rather than to bother updating these stupid IWYU-generated comments, you may as well remove them as you go. ::: gfx/layers/client/CompositableClient.cpp @@ +47,5 @@ > + mCompositableClient->mCompositableChild = nullptr; > + } > + } > + > + CompositableClient* mCompositableClient; Even though this is local to a cpp file, I still appreciate having data members private. Especially as they used to be private. Up to you.
Attachment #8409335 -
Flags: review?(bjacob) → review+
Comment 5•10 years ago
|
||
Comment on attachment 8409338 [details] [diff] [review] 2) Move CompositableParent to the .cpp Review of attachment 8409338 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/composite/CompositableHost.cpp @@ +58,5 @@ > + mHost->Detach(nullptr, CompositableHost::FORCE_DETACH); > + } > + } > + > + RefPtr<CompositableHost> mHost; Same comment about members being private, as on the previous patch. Up to you.
Attachment #8409338 -
Flags: review?(bjacob) → review+
Assignee | ||
Comment 6•10 years ago
|
||
landed part 1) https://hg.mozilla.org/integration/mozilla-inbound/rev/dc10c7c748ee
Whiteboard: [leave open]
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dc10c7c748ee
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/241760451193
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8412689 -
Flags: review?(bjacob)
Updated•10 years ago
|
Attachment #8412689 -
Flags: review?(bjacob) → review+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f727dc726d3d
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/f727dc726d3d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 13•10 years ago
|
||
Benoit, comment 12 landed for 32, but comment 10 landed for 31. Do you think we should land the follow-up on Aurora too? I've confirmed that it uplifts cleanly. Nicolas is away, so if you don't mind, can you please request it if you agree? :)
Flags: needinfo?(bjacob)
Comment 14•10 years ago
|
||
It's not worth requesting uplift for this. Comment 12 is a nice fixup as it prevents slowing down compiles with excessive includes (a #include is more expensive if it is itself in a header file), and it prevents accidentally relying on these #includes that we now consider as implementation details (whence the move to .cpp files) but neither of these benefits is very meaningful on aurora and stable channels. That's mostly beneficial on mozilla-central for active development. Note, if it compiles cleanly, it's a very safe patch to uplift (the only risk was potentially failing to compile). Just not really worth it.
Flags: needinfo?(bjacob)
You need to log in
before you can comment on or make changes to this bug.
Description
•