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

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

Index: third_party/WebKit/Source/core/events/InputEvent.cpp
diff --git a/third_party/WebKit/Source/core/events/InputEvent.cpp b/third_party/WebKit/Source/core/events/InputEvent.cpp
index 337496ae8c03ae9750243fa347e79f0160a22ed1..533d439220ba4f75b94268f763631a282b97e06c 100644
--- a/third_party/WebKit/Source/core/events/InputEvent.cpp
+++ b/third_party/WebKit/Source/core/events/InputEvent.cpp
@@ -4,6 +4,7 @@
 
 #include "core/events/InputEvent.h"
 
+#include "core/dom/Range.h"
 #include "core/events/EventDispatcher.h"
 #include "public/platform/WebEditingCommandType.h"
 
@@ -62,10 +63,12 @@ InputEvent::InputEvent(const AtomicString& type, const InputEventInit& initializ
         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;
 
@@ -76,6 +79,8 @@ InputEvent* InputEvent::createBeforeInput(InputType inputType, const String& dat
     inputEventInit.setInputType(convertInputTypeToString(inputType));
     inputEventInit.setData(data);
     inputEventInit.setIsComposing(isComposing == IsComposing);
+    if (ranges)
+        inputEventInit.setRanges(*ranges);
 
     return InputEvent::create(EventTypeNames::beforeinput, inputEventInit);
 }
@@ -90,9 +95,42 @@ 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