Index: third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl |
diff --git a/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl b/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl |
index 28006cabf0a4aea5176a84566b9fe40aa7936b2f..ceb54e4e516b9b7d5a324c0f2dede122c60a5219 100644 |
--- a/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl |
+++ b/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl |
@@ -46,12 +46,55 @@ struct SameSizeAsComputedStyleBase { |
// The generated portion of ComputedStyle. For more info, see the header comment |
// in ComputedStyle.h. |
- |
-// ComputedStyleBase is a templated class to allow it to use functions |
-// on ComputedStyle. This allows ComputedStyleBase to use hand written |
-// functions it would otherwise not know about. |
-// It should only be templated with the ComputedStyle class and no other class |
-// is allowed. |
+// |
+// ComputedStyleBase is a generated class that implements the storage aspect of |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
I find this paragraph unclear, I think particularl
shend
2017/05/17 23:30:00
Done.
|
+// ComputedStyle. Conceptually, it stores a set of data members or 'fields' |
+// that are used in ComputedStyle. These fields can represent CSS properties or |
+// internal style information. |
+// |
+// However, storing every data member directly in this class is wasteful, since |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
On a first read through, it's not clear why this i
shend
2017/05/17 23:30:00
Rewrote the paragraph.
|
+// not every data member is used frequently. Thus, we group fields into |
+// nested classes inside ComputedStyleBase and store pointers to |
+// these groups instead. For example, 'width' and 'height' are stored in a group |
+// called StyleBoxData. ComputedStyleBase stores a pointer to an instance of |
+// StyleBoxData, so different multiple ComputedStyleBases can save memory by |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
"different multiple" is redundant - pick just one
shend
2017/05/17 23:30:00
Done.
|
+// sharing the same instance of StyleBoxData. Furthermore, these nested classes |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
I think put a paragraph break here and have the ne
shend
2017/05/17 23:30:00
Omitted discussion of nested classes, since it's a
|
+// can themselves contain nested classes and so on, meaning we can view the |
+// storage of ComputedStyleBase as a tree structure of groups with |
+// ComputedStyleBase at the root: |
+// |
+// ComputedStyleBase |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
I wouldn't bother listing all of these groups, esp
shend
2017/05/17 23:30:00
Done.
|
+// |- StyleSurroundData |
+// |- StyleBackgroundData |
+// |- StyleVisualData |
+// |- StyleBoxData |
+// |- StyleInheritedData |
+// |- StyleRareInheritedData |
+// |- StyleRareNonInheritedData |
+// |- StyleDeprecatedFlexibleBoxData |
+// |- StyleFlexibleBoxData |
+// |- StyleMultiColData |
+// |- StyleTransformData |
+// |- StyleWillChangeData |
+// |- StyleGridData |
+// |- StyleGridItemData |
+// |- StyleScrollSnapData |
+// |
+// where every field belongs to exactly one of these groups. ComputedStyle |
+// should never refer to these groups, since they may changed if we find a |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
I misinterpreted this sentence the first time thro
shend
2017/05/17 23:30:00
Removed this sentence as it is not worth the confu
|
+// better layout structure. |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
There's no need to specify a reason why they may h
shend
2017/05/17 23:30:00
Done.
|
+// |
+// Not every field has the same interface. The interface for a field is |
+// determined by its 'template'. Different field templates result in slightly |
+// different accessors and helpers. For example, a field with the 'keyword' |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
I don't think it's necessary to specify "Different
shend
2017/05/17 23:30:00
Done.
|
+// template has only one setter, whereas an 'external' field has an extra setter |
+// that takes an rvalue reference. A list of the available templates can be |
+// found in CSSProperties.json5. |
+// |
+// ComputedStyleBase is a template class to allow it to use functions on |
+// ComputedStyle. This allows ComputedStyleBase to use hand written functions it |
+// would otherwise not know about. It should only be templated with the |
+// ComputedStyle class and no other class is allowed. |
suzyh_UTC10 (ex-contributor)
2017/05/17 09:01:27
Consider adding a TODO (ideally with bug reference
shend
2017/05/17 23:30:00
Added TODO, but no bug as it is a very small chang
suzyh_UTC10 (ex-contributor)
2017/05/19 02:33:59
The main reason to add a bug is not because a TODO
|
template <class ComputedStyleFinal> |
class CORE_EXPORT ComputedStyleBase { |
public: |