Make NGInlineItemsBuilder construct whitespace-collapsed offset mapping

third_party/WebKit/Source/core/layout/ng/inline/ng_inline_items_builder.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 NGInlineItemsBuilder_h
 #define NGInlineItemsBuilder_h
 
 #include "core/CoreExport.h"
 #include "core/layout/ng/inline/ng_inline_node.h"
 #include "platform/wtf/Allocator.h"
 #include "platform/wtf/Vector.h"
 #include "platform/wtf/text/StringBuilder.h"
 #include "platform/wtf/text/WTFString.h"
 
 namespace blink {
 
 class ComputedStyle;
 class LayoutObject;
 class NGInlineItem;
 
 // NGInlineItemsBuilder builds a string and a list of NGInlineItem from inlines.
 //
 // When appending, spaces are collapsed according to CSS Text, The white space
 // processing rules
 // https://drafts.csswg.org/css-text-3/#white-space-rules
 //
 // By calling EnterInline/ExitInline, it inserts bidirectional control
 // characters as defined in:
 // https://drafts.csswg.org/css-writing-modes-3/#bidi-control-codes-injection-table
+//
+// Optionally, NGInlineItemsBuilder takes parameter |collapsed_indexes| to
+// return how spaces are collapsed. For each string appended to
+// NGInlineItemsBuilder, a Vector<unsigned> will be added to
+// |collapsed_indexes|, storing the indexes of all collapsed spaces in sorted
+// order. See the unit tests for examples.
 class CORE_EXPORT NGInlineItemsBuilder {
   STACK_ALLOCATED();
 
  public:
-  explicit NGInlineItemsBuilder(Vector<NGInlineItem>* items) : items_(items) {}
+  explicit NGInlineItemsBuilder(Vector<NGInlineItem>* items)
+      : items_(items), collapsed_indexes_(nullptr) {}
+  explicit NGInlineItemsBuilder(Vector<NGInlineItem>* items,
+                                Vector<Vector<unsigned>>* collapsed_indexes)
+      : items_(items), collapsed_indexes_(collapsed_indexes) {}
   ~NGInlineItemsBuilder();
 
   String ToString();
 
   void SetIsSVGText(bool value) { is_svgtext_ = value; }
 
   // Returns whether the items contain any Bidi controls.
   bool HasBidiControls() const { return has_bidi_controls_; }
 
   // Append a string.
@@ skipping 40 matching lines @@
     UChar character;
   } OnExitNode;
   Vector<OnExitNode> exits_;
 
   enum class CollapsibleSpace { kNone, kSpace, kNewline };
 
   CollapsibleSpace last_collapsible_space_ = CollapsibleSpace::kSpace;
   bool is_svgtext_ = false;
   bool has_bidi_controls_ = false;
 
+  // When provided, the indexes of collapsed spaces in each appended string will
+  // be eventually added to |collapsed_indexes_|.
+  Vector<Vector<unsigned>>* collapsed_indexes_;

[yosin_UTC9, 2017/06/21 04:22:41]

Could you introduce OffsetMappingBuilder class to hold |collapsed_indexes_| and
|input_strings_length_| and
member functions to update them in another file?
We would like to bold editing related part in core/layout for ease of
maintenance.

e.g.
  if (collapsed_indexes_) {
    input_strings_lengths_.push_back(string.length());
    collapsed_indexes_->emplace_back();
  }
=>
  if (offset_builder_)
    offset_builder_->Append(string);


  if (collapsed_indeexs_)
    collapsed_indexes_->back().push_back(i);
=>
   if (offset_builder_)
    offset_builder_->Append(i);

These names are just example. 
Please use appropriate name for member functions.

[kojii, 2017/06/21 09:47:13]

I take this opposite. Since this is tightly coupled with this class, separating
out will make us harder to change this class. Higher-level classes/logic can
live in editing.

Agree on better to have a struct/class of a vector, rather than two vector, but
I haven't understood why we need a vector of vector for each append. Isn't what
we need just a map of character indexes?

If possible, I'd like to avoid editing code depends on how we call Append(), but
you must have some needs for this data, can you explain?

[Xiaocheng, 2017/06/21 18:15:06]

I agree with yosin@ that it's a better pattern if we take an
OffsetMappingBuilder as the optional parameter instead of |collapsed_indexes|.
With the builder, we can return the collapsing information in a more flexible
way, and allows the builder to utilize the information more easily. I'll update
the patch to see how it works.
The current design is: for each string appended, we need a vector to store the
indexes of the collapsed spaces. So we need a vector of vector for all the
strings together. An offset mapping builder will take this vector of vector and
build the mapping.
I'll see if the dependence can be reduced in the updated patch.

+  // Length of each appended string. Used only for computing the indexes of
+  // collapsed spaces when |collapsed_indexes_| is provided.
+  Vector<unsigned> input_strings_lengths_;
+
   void AppendWithWhiteSpaceCollapsing(const String&,
                                       unsigned start,
                                       unsigned end,
                                       const ComputedStyle*,
                                       LayoutObject*);
   void AppendWithoutWhiteSpaceCollapsing(const String&,
                                          const ComputedStyle*,
                                          LayoutObject*);
   void AppendWithPreservingNewlines(const String&,
                                     const ComputedStyle*,
                                     LayoutObject*);
 
-  void AppendForcedBreak(const ComputedStyle*, LayoutObject*);
+  void AppendForcedBreak(const ComputedStyle*, LayoutObject*, unsigned);
 
   // Because newlines may be removed depends on following characters, newlines
   // at the end of input string is not added to |text_| but instead
   // |has_pending_newline_| flag is set.
   // This function determines whether to add the newline or ignore.
   void ProcessPendingNewline(const String&, const ComputedStyle*);
 
   // Removes the collapsible space at the end of |text_| if exists.
-  void RemoveTrailingCollapsibleSpaceIfExists(unsigned*);
-  void RemoveTrailingCollapsibleSpace(unsigned*);
+  void RemoveTrailingCollapsibleSpaceIfExists(unsigned*, unsigned);
+  void RemoveTrailingCollapsibleSpace(unsigned*, unsigned);
 
   void RemoveTrailingCollapsibleNewlineIfNeeded(unsigned*,
                                                 const String&,
                                                 unsigned,
                                                 const ComputedStyle*);
 
+  void AddLastTrailingSpaceToRemovedIndexes(unsigned);
+
   void Enter(LayoutObject*, UChar);
   void Exit(LayoutObject*);
 };
 
 }  // namespace blink
 
 #endif  // NGInlineItemsBuilder_h