Closed Bug 1440141 Opened 6 years ago Closed 6 years ago

stylo: Fix stylo-only build

Categories

(Core :: CSS Parsing and Computation, enhancement, P2)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

(Keywords: meta)

Attachments

(3 files)

Bug 1440141 part 1 - Move GeckoStyleContext::TakeRef to nsStyleContextInlines.h so that header doesn't depend on the Inlines.h.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1440141 part 2 - Move some headers around.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Bug 1440141 part 3 - Avoid invoking GetAsGecko when old style system is disabled.
59 bytes, text/x-review-board-request
emilio
: review+
Details 
It was broken by bug 1438020.
Assignee: nobody → xidorn+moz
Hmmm... why am I hitting a build failure on an old code now :/
I cannot explain why part 3 is necessary or why it wasn't necessary when we landed bug 1430014... I indeed had a green try push back to that day.
Comment on attachment 8952907 [details]
Bug 1440141 part 2 - Move some headers around.

https://reviewboard.mozilla.org/r/222158/#review228088

::: layout/style/StyleSheet.cpp
(Diff revision 1)
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "mozilla/StyleSheet.h"
>  
>  #include "nsStyleContext.h"
> -#include "nsStyleContextInlines.h"

As I noted at the end of bug 1438020 comment 17: we still need this nsStyleContextInlines.h here (or in GeckoStyleContext.h). Otherwise we end up introducing the "warning: inline function 'nsStyleContext::AsGecko' is not defined [-Wundefined-inline]" noted in an earlier comment there, in builds with unification disabled. (Not the default configuration, but still worth heeding as a sanity-check for dependency-satisfaction purposes.)

(BTW, I suspect most #includers of GeckoStyleContext.h end up including the "nsStyleContextInlines.h" header indirectly, by virtue of including nsIFrame.h or WritingModes.h.  Both of those headers include nsStyleContextInlines.h, which is kind of gross, but is the current state of the world.)
Attachment #8952907 - Flags: review?(dholbert) → review-
Comment on attachment 8952907 [details]
Bug 1440141 part 2 - Move some headers around.

https://reviewboard.mozilla.org/r/222158/#review228088

> As I noted at the end of bug 1438020 comment 17: we still need this nsStyleContextInlines.h here (or in GeckoStyleContext.h). Otherwise we end up introducing the "warning: inline function 'nsStyleContext::AsGecko' is not defined [-Wundefined-inline]" noted in an earlier comment there, in builds with unification disabled. (Not the default configuration, but still worth heeding as a sanity-check for dependency-satisfaction purposes.)
> 
> (BTW, I suspect most #includers of GeckoStyleContext.h end up including the "nsStyleContextInlines.h" header indirectly, by virtue of including nsIFrame.h or WritingModes.h.  Both of those headers include nsStyleContextInlines.h, which is kind of gross, but is the current state of the world.)

In part 1, I moved the function which depends on `AsGecko` into `nsStyleContextInlines.h`.
Attachment #8952907 - Flags: review- → review?(dholbert)
Comment on attachment 8952907 [details]
Bug 1440141 part 2 - Move some headers around.

https://reviewboard.mozilla.org/r/222158/#review228088

> In part 1, I moved the function which depends on `AsGecko` into `nsStyleContextInlines.h`.

Ah, I missed that part. OK, I'll retest with both patches to double-check.
Comment on attachment 8952907 [details]
Bug 1440141 part 2 - Move some headers around.

https://reviewboard.mozilla.org/r/222158/#review228110

(For anyone not watching in IRC: I was still getting the build warnings, even with all three original patches here. hence this revision.)

This updated version works for me. Thanks! r+
Attachment #8952907 - Flags: review?(dholbert) → review+
Comment on attachment 8952906 [details]
Bug 1440141 part 1 - Move GeckoStyleContext::TakeRef to nsStyleContextInlines.h so that header doesn't depend on the Inlines.h.

https://reviewboard.mozilla.org/r/222156/#review228128

::: layout/style/GeckoStyleContext.h:17
(Diff revision 1)
>  namespace mozilla {
>  
>  class GeckoStyleContext final : public nsStyleContext {
>  public:
> -  static already_AddRefed<GeckoStyleContext>
> -  TakeRef(already_AddRefed<nsStyleContext> aStyleContext)
> +  static inline already_AddRefed<GeckoStyleContext>
> +  TakeRef(already_AddRefed<nsStyleContext> aStyleContext);

Should this also be `#ifdef`d? Getting a link error for using this instead of a compiler error earlier sounds not great.
Attachment #8952906 - Flags: review?(emilio) → review+
Comment on attachment 8952908 [details]
Bug 1440141 part 3 - Avoid invoking GetAsGecko when old style system is disabled.

https://reviewboard.mozilla.org/r/222160/#review228130
Attachment #8952908 - Flags: review?(emilio) → review+
Comment on attachment 8952906 [details]
Bug 1440141 part 1 - Move GeckoStyleContext::TakeRef to nsStyleContextInlines.h so that header doesn't depend on the Inlines.h.

https://reviewboard.mozilla.org/r/222156/#review228128

> Should this also be `#ifdef`d? Getting a link error for using this instead of a compiler error earlier sounds not great.

This whole file is excluded when `MOZ_OLD_STYLE` is not set: https://searchfox.org/mozilla-central/rev/9a8d3f8191b15cdca89a7ce044c7bea2dd0462dc/layout/style/moz.build#146
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1f351b97729
part 1 - Move GeckoStyleContext::TakeRef to nsStyleContextInlines.h so that header doesn't depend on the Inlines.h. r=emilio
https://hg.mozilla.org/integration/autoland/rev/2cdeb899bb11
part 2 - Move some headers around. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/c1c444858b32
part 3 - Avoid invoking GetAsGecko when old style system is disabled. r=emilio
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: