[InputEvent] Support |sequence<Range> getRanges()| in 'beforeinput'

third_party/WebKit/Source/core/events/InputEvent.cpp

 // 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.
 
 #include "core/events/InputEvent.h"
 
+#include "core/dom/Range.h"
 #include "core/events/EventDispatcher.h"
 #include "public/platform/WebEditingCommandType.h"
 
 namespace blink {
 
 namespace {
 
 const struct {
     InputEvent::InputType inputType;
     const char* stringName;
@@ skipping 38 matching lines @@
     : UIEvent(type, initializer)
 {
     // TODO(ojan): We should find a way to prevent conversion like String->enum->String just in order to use initializer.
     // See InputEvent::createBeforeInput() for the first conversion.
     if (initializer.hasInputType())
         m_inputType = convertStringToInputType(initializer.inputType());
     if (initializer.hasData())
         m_data = initializer.data();
     if (initializer.hasIsComposing())
         m_isComposing = initializer.isComposing();
+    if (initializer.hasRanges())
+        m_ranges = initializer.ranges();
 }
 
 /* static */
-InputEvent* InputEvent::createBeforeInput(InputType inputType, const String& data, EventCancelable cancelable, EventIsComposing isComposing)
+InputEvent* InputEvent::createBeforeInput(InputType inputType, const String& data, EventCancelable cancelable, EventIsComposing isComposing, const RangeVector* ranges)
 {
     InputEventInit inputEventInit;
 
     inputEventInit.setBubbles(true);
     inputEventInit.setCancelable(cancelable == IsCancelable);
     // TODO(ojan): We should find a way to prevent conversion like String->enum->String just in order to use initializer.
     // See InputEvent::InputEvent() for the second conversion.
     inputEventInit.setInputType(convertInputTypeToString(inputType));
     inputEventInit.setData(data);
     inputEventInit.setIsComposing(isComposing == IsComposing);
+    if (ranges)
+        inputEventInit.setRanges(*ranges);
 
     return InputEvent::create(EventTypeNames::beforeinput, inputEventInit);
 }
 
 String InputEvent::inputType() const
 {
     return convertInputTypeToString(m_inputType);
 }
 
 bool InputEvent::isInputEvent() const
 {
     return true;
 }
 
+EventDispatchMediator* InputEvent::createMediator()

[ojan, 2016/05/31 22:56:48]

At some point, we should get rid of this EventDispatchMediator pattern and do
something a bit simpler. For example, the InputEvent could have
beforeDispatchEvent and afterDispatchEvent virtual methods that dispatchEvent
calls instead. Can you put a TODO for now?

+{
+    return InputEventDispatchMediator::create(this);
+}
+
 DEFINE_TRACE(InputEvent)
 {
     UIEvent::trace(visitor);
+    visitor->trace(m_ranges);
+}
+
+InputEventDispatchMediator* InputEventDispatchMediator::create(InputEvent* inputEvent)
+{
+    return new InputEventDispatchMediator(inputEvent);
+}
+
+InputEventDispatchMediator::InputEventDispatchMediator(InputEvent* inputEvent)
+    : EventDispatchMediator(inputEvent)
+{
+}
+
+InputEvent& InputEventDispatchMediator::event() const
+{
+    return toInputEvent(EventDispatchMediator::event());
+}
+
+DispatchEventResult InputEventDispatchMediator::dispatchEvent(EventDispatcher& dispatcher) const
+{
+    DispatchEventResult result = dispatcher.dispatch();
+    // JavaScript might hold event objects (e.g. for custom redo/undo stack), but
+    // for performance measurement we don't want to implicitly maintain a bunch
+    // of |Range| objects.
+    // Here we clear |m_ranges| from event object after dispatch, JavaScript should
+    // explicitly call |getRanges()|->|toRange()| if they want to keep a copy of |Range|.

[ojan, 2016/05/31 22:56:48]

I see. I reread the meeting notes for this.
https://docs.google.com/document/d/1hCj6QX77NYIVY0RWrMHT1Yra6t8_Qu8PopaWLG0AM...

It's not about memory or performance measurement exactly. There's a few things
that went into this:
1. We don't want to expose live Range objects for the author to hold onto
because they are live, so they slow down all DOM operations. So, beforeInput
doesn't have a way of getting Ranges, just StaticRanges.
2. Naive code that calls getRanges will do the wrong thing if one beforeInput
callback modifies the range that's being acted on before the next one that gets
called.

So the solution was to have a getRanges method that computes the range based off
the set of internally stored Ranges. But, we don't want to hold on to those
Ranges indefinitely to avoid #1, so we clear them.

This will be a weird part of this API, but I don't have a better suggestion at
the moment.

Please update the comment. Otherwise, looks good.

+    event().m_ranges.clear();
+    return result;
 }
 
 } // namespace blink