Use NGLogicalRect instead of NGExclusion for exclusions.

third_party/WebKit/Source/core/layout/ng/ng_units.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NGUnits_h
 #define NGUnits_h
 
 #include "core/CoreExport.h"
 #include "core/layout/ng/ng_direction.h"
 #include "core/layout/ng/ng_writing_mode.h"
@@ skipping 74 matching lines @@
 struct NGPhysicalLocation {
   LayoutUnit left;
   LayoutUnit top;
 };
 
 struct NGPhysicalRect {
   NGPhysicalOffset offset;
   NGPhysicalSize size;
 };
 
+// TODO(glebl): move to a separate file in layout/ng/units.
 struct CORE_EXPORT NGLogicalRect {
   NGLogicalRect() {}
+  // TODO(glebl): Make this private and use the builder everywhere.
   NGLogicalRect(LayoutUnit inline_offset,
                 LayoutUnit block_offset,
                 LayoutUnit inline_size,
                 LayoutUnit block_size)
       : offset(inline_offset, block_offset), size(inline_size, block_size) {}
 
   bool IsEmpty() const;
+
+  // Whether this rectangle within the provided rectangle.
+  bool IsWithin(const NGLogicalRect& other) const;
+
   String ToString() const;
   bool operator==(const NGLogicalRect& other) const;
 
+  // Getters
+  LayoutUnit InlineStartOffset() const { return offset.inline_offset; }
+
+  LayoutUnit InlineEndOffset() const {
+    return offset.inline_offset + size.inline_size;
+  }
+
+  LayoutUnit BlockStartOffset() const { return offset.block_offset; }
+
+  LayoutUnit BlockEndOffset() const {
+    return offset.block_offset + size.block_size;
+  }
+
+  LayoutUnit BlockSize() const { return size.block_size; }
+
+  LayoutUnit InlineSize() const { return size.inline_size; }
+
+  class Builder {

[ikilpatrick, 2016/10/28 23:18:24]

yeah, i'm not sold on using the builder for these types. but would like others
opinions :)

[Gleb Lanbin, 2016/10/28 23:54:09]

yes, I'm not a 100% fan of it either because it's too verbose. That's why I
added a TODO to move it in a separate file. In google3 this boilerplate code for
builders would be pre-generated.

just want to list other variants that were considered.
1) Use constructor.
Example:
new NGExclusion(LayoutUnit(0), LayoutUnit(600), LayoutUnit(100),
LayoutUnit(500));
Cons: impossible to read and error-prone. It's not clear what is set to what.
Pros: concise and already available

2) Designated Initializers
Example: {.inline_offset = LayoutUnit(50), .block_offset = LayoutUnit(50)} etc.
Cons: easy to read, concise and already available
Pros: it's a part of C99 standard. Our MSVC 2015 doesn't support it
unfortunately.

3) Comments
Example: {/* inline_offset */ LayoutUnit(50), /* block_offset */LayoutUnit(50)}
etc.
Cons: easy to read, concise and already available
Pros: it's hard to enforce this rule if it's not a part of API or style rules.

4) Builders + Comments.
For small structs lets use comments. For big structs lets use builders. 
NGLogicalOffset, NGLogicalSize should also have builders

in this case we can split this builder.
Example:
NGLogicalRect::Builder()
    .SetOffset(
        NGLogicalOffset::Builder()
        .SetInlineOffset(bla)
        .SetBlocOffset(bla).Build())
    .SetSize(
        NGLogicalSize::Builder()
        .SetInlineSize(bla)
        .SetBlockSize(bla).Build())
    .Build()

+   public:
+    Builder() = default;
+
+    Builder& SetInlineOffset(const LayoutUnit& inline_offset) {
+      inline_offset_ = inline_offset;
+      return *this;
+    }
+
+    Builder& SetBlockOffset(const LayoutUnit& block_offset) {
+      block_offset_ = block_offset;
+      return *this;
+    }
+
+    Builder& SetInlineSize(const LayoutUnit& inline_size) {
+      inline_size_ = inline_size;
+      return *this;
+    }
+
+    Builder& SetBlockSize(const LayoutUnit& block_size) {
+      block_size_ = block_size;
+      return *this;
+    }
+
+    NGLogicalRect* BuildNew() {
+      return new NGLogicalRect(inline_offset_, block_offset_, inline_size_,
+                               block_size_);
+    }
+
+    NGLogicalRect Build() {
+      return NGLogicalRect(inline_offset_, block_offset_, inline_size_,
+                           block_size_);
+    }
+
+   private:
+    LayoutUnit inline_offset_;
+    LayoutUnit block_offset_;
+    LayoutUnit inline_size_;
+    LayoutUnit block_size_;
+    DISALLOW_COPY_AND_ASSIGN(Builder);
+  };
+
   NGLogicalOffset offset;
   NGLogicalSize size;
 };
 
 inline std::ostream& operator<<(std::ostream& stream,
                                 const NGLogicalRect& value) {
   return stream << value.ToString();
 }
 
 struct NGPixelSnappedPhysicalRect {
@@ skipping 74 matching lines @@
 
 // Struct to represent a simple edge that has start and end.
 struct NGEdge {
   LayoutUnit start;
   LayoutUnit end;
 };
 
 }  // namespace blink
 
 #endif  // NGUnits_h