Make RenderObject::style() return a const object

Source/core/layout/LayoutObject.h

 /*
  * Copyright (C) 2000 Lars Knoll (knoll@kde.org)
  *           (C) 2000 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  *           (C) 2004 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 583 matching lines @@
     bool hasHiddenBackface() const { return style() && style()->backfaceVisibility() == BackfaceVisibilityHidden; }
 
     bool hasFilter() const { return style() && style()->hasFilter(); }
 
     bool hasShapeOutside() const { return style() && style()->shapeOutside(); }
 
     inline bool preservesNewline() const;
 
     // The pseudo element style can be cached or uncached.  Use the cached method if the pseudo element doesn't respect
     // any pseudo classes (and therefore has no concept of changing state).
-    LayoutStyle* getCachedPseudoStyle(PseudoId, LayoutStyle* parentStyle = 0) const;
-    PassRefPtr<LayoutStyle> getUncachedPseudoStyle(const PseudoStyleRequest&, LayoutStyle* parentStyle = 0, LayoutStyle* ownStyle = 0) const;
+    LayoutStyle* getCachedPseudoStyle(PseudoId, const LayoutStyle* parentStyle = 0) const;
+    PassRefPtr<LayoutStyle> getUncachedPseudoStyle(const PseudoStyleRequest&, const LayoutStyle* parentStyle = 0, const LayoutStyle* ownStyle = 0) const;
 
     virtual void updateDragState(bool dragOn);
 
     RenderView* view() const { return document().renderView(); };
     FrameView* frameView() const { return document().view(); };
 
     bool isRooted() const;
 
     Node* node() const
     {
@@ skipping 175 matching lines @@
     virtual IntRect absoluteFocusRingBoundingBoxRect() const;
 
     static FloatRect absoluteBoundingBoxRectForRange(const Range*);
 
     // the rect that will be painted if this object is passed as the paintingRoot
     LayoutRect paintingRootRect(LayoutRect& topLevelRect);
 
     virtual LayoutUnit minPreferredLogicalWidth() const { return 0; }
     virtual LayoutUnit maxPreferredLogicalWidth() const { return 0; }
 
-    LayoutStyle* style() const { return m_style.get(); }
+    const LayoutStyle* style() const { return m_style.get(); }
+    LayoutStyle* mutableStyle() const { return m_style.get(); }
 
     // m_style can only be nullptr before the first style is set, thus most
     // callers will never see a nullptr style and should use styleRef().
     // FIXME: It would be better if style() returned a const reference.
     const LayoutStyle& styleRef() const { return mutableStyleRef(); }
     LayoutStyle& mutableStyleRef() const { ASSERT(m_style); return *m_style; }
 
     /* The following methods are inlined in LayoutObjectInlines.h */
-    LayoutStyle* firstLineStyle() const;
+    const LayoutStyle* firstLineStyle() const;
     const LayoutStyle& firstLineStyleRef() const;
-    LayoutStyle* style(bool firstLine) const;
+    const LayoutStyle* style(bool firstLine) const;
     const LayoutStyle& styleRef(bool firstLine) const;

[rune, 2015/02/10 12:03:01]

Why do you need the pointer-return variants?

I assume the pointer is always expected to be non-null since you have reference
returns for the same member?

Is this to avoid a larger size of the patch? work in progress?

[rune, 2015/02/10 12:03:01]

Do we really need the pointer-return variants?

Which are the callers mentioned above that can expect a nullptr?

Would it be possible to hide those accesses inside the LayoutObject
implementation so that we can get away with reference getters only (keeping the
assert in the ref-getter)?

[Julien - ping for review, 2015/02/10 23:10:43]

We do. The reason is that there is a small window during which style() can
return nullptr.
It should be any code that calls style() before the first setStyleInternal() is
called (in particular styleWillChange)
That's a good question and I don't have a definite answer to that. That's
because I haven't investigated all the callers to see if they *really* need a
nullptr check. I would expect most of these to be unneeded and your proposal
make sense but it's just a hunch based on the short window for a nullptr style.

This is a good follow-up that I would gladly try :)

 
     static inline Color resolveColor(const LayoutStyle& styleToUse, int colorProperty)
     {
         return styleToUse.visitedDependentColor(colorProperty);
     }
 
     inline Color resolveColor(int colorProperty) const
     {
         return style()->visitedDependentColor(colorProperty);
     }
@@ skipping 798 matching lines @@
 void showTree(const blink::LayoutObject*);
 void showLineTree(const blink::LayoutObject*);
 void showRenderTree(const blink::LayoutObject* object1);
 // We don't make object2 an optional parameter so that showRenderTree
 // can be called from gdb easily.
 void showRenderTree(const blink::LayoutObject* object1, const blink::LayoutObject* object2);
 
 #endif
 
 #endif // LayoutObject_h