Commit dca621cb authored by Anupam Snigdha's avatar Anupam Snigdha Committed by Commit Bot

Fixed use after poison and memory leaks in EditContext

-The Trace method was missing EventTargetWithInlineData::Trace call
-Using ActiveScriptWrappable now so EditContext wrapper object remains alive
till the execution context is valid and when the composition events arrive,
they can be processed properly by the event listeners for the EditContext.
- Also removed an invalid test case that had a global reference to EditContext
which was causing memory leaks

Test: run_web_tests web_tests/editing/input/edit-context.html

Bug: 1023351, 1023242

Change-Id: If7e9d7ebea195095b0ac5048cd0864d3e0b730f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1911280Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Commit-Queue: Anupam Snigdha <snianu@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#714523}
parent 0d2eb2d4
...@@ -24,7 +24,7 @@ ...@@ -24,7 +24,7 @@
namespace blink { namespace blink {
EditContext::EditContext(ScriptState* script_state, const EditContextInit* dict) EditContext::EditContext(ScriptState* script_state, const EditContextInit* dict)
: ContextClient(ExecutionContext::From(script_state)) { : ContextLifecycleObserver(ExecutionContext::From(script_state)) {
DCHECK(IsMainThread()); DCHECK(IsMainThread());
if (dict->hasText()) if (dict->hasText())
...@@ -58,26 +58,30 @@ const AtomicString& EditContext::InterfaceName() const { ...@@ -58,26 +58,30 @@ const AtomicString& EditContext::InterfaceName() const {
} }
ExecutionContext* EditContext::GetExecutionContext() const { ExecutionContext* EditContext::GetExecutionContext() const {
return ContextClient::GetExecutionContext(); return ContextLifecycleObserver::GetExecutionContext();
}
bool EditContext::HasPendingActivity() const {
return GetExecutionContext() && HasEventListeners();
} }
InputMethodController& EditContext::GetInputMethodController() const { InputMethodController& EditContext::GetInputMethodController() const {
return ContextClient::GetFrame()->GetInputMethodController(); return ContextLifecycleObserver::GetFrame()->GetInputMethodController();
} }
void EditContext::DispatchCompositionEndEvent(const String& text) { void EditContext::DispatchCompositionEndEvent(const String& text) {
auto* event = MakeGarbageCollected<CompositionEvent>( auto* event = MakeGarbageCollected<CompositionEvent>(
event_type_names::kCompositionend, ContextClient::GetFrame()->DomWindow(), event_type_names::kCompositionend,
text); ContextLifecycleObserver::GetFrame()->DomWindow(), text);
DispatchEvent(*event); DispatchEvent(*event);
} }
bool EditContext::DispatchCompositionStartEvent(const String& text) { bool EditContext::DispatchCompositionStartEvent(const String& text) {
auto* event = MakeGarbageCollected<CompositionEvent>( auto* event = MakeGarbageCollected<CompositionEvent>(
event_type_names::kCompositionstart, event_type_names::kCompositionstart,
ContextClient::GetFrame()->DomWindow(), text); ContextLifecycleObserver::GetFrame()->DomWindow(), text);
DispatchEvent(*event); DispatchEvent(*event);
if (!ContextClient::GetFrame()) if (!ContextLifecycleObserver::GetFrame())
return false; return false;
return true; return true;
} }
...@@ -573,8 +577,9 @@ WebRange EditContext::GetSelectionOffsets() const { ...@@ -573,8 +577,9 @@ WebRange EditContext::GetSelectionOffsets() const {
} }
void EditContext::Trace(Visitor* visitor) { void EditContext::Trace(Visitor* visitor) {
EventTarget::Trace(visitor); ActiveScriptWrappable::Trace(visitor);
ScriptWrappable::Trace(visitor); ContextLifecycleObserver::Trace(visitor);
ContextClient::Trace(visitor); EventTargetWithInlineData::Trace(visitor);
} }
} // namespace blink } // namespace blink
...@@ -11,10 +11,10 @@ ...@@ -11,10 +11,10 @@
#include "third_party/blink/public/platform/web_text_input_type.h" #include "third_party/blink/public/platform/web_text_input_type.h"
#include "third_party/blink/public/web/web_ime_text_span.h" #include "third_party/blink/public/web/web_ime_text_span.h"
#include "third_party/blink/public/web/web_input_method_controller.h" #include "third_party/blink/public/web/web_input_method_controller.h"
#include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
#include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/dom/events/event_target.h" #include "third_party/blink/renderer/core/dom/events/event_target.h"
#include "third_party/blink/renderer/core/execution_context/context_lifecycle_observer.h" #include "third_party/blink/renderer/core/execution_context/context_lifecycle_observer.h"
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
namespace blink { namespace blink {
...@@ -32,7 +32,8 @@ class InputMethodController; ...@@ -32,7 +32,8 @@ class InputMethodController;
// https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/master/EditContext/explainer.md // https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/master/EditContext/explainer.md
class CORE_EXPORT EditContext final : public EventTargetWithInlineData, class CORE_EXPORT EditContext final : public EventTargetWithInlineData,
public ContextClient, public ActiveScriptWrappable<EditContext>,
public ContextLifecycleObserver,
public WebInputMethodController { public WebInputMethodController {
DEFINE_WRAPPERTYPEINFO(); DEFINE_WRAPPERTYPEINFO();
USING_GARBAGE_COLLECTED_MIXIN(EditContext); USING_GARBAGE_COLLECTED_MIXIN(EditContext);
...@@ -134,9 +135,12 @@ class CORE_EXPORT EditContext final : public EventTargetWithInlineData, ...@@ -134,9 +135,12 @@ class CORE_EXPORT EditContext final : public EventTargetWithInlineData,
void setInputPanelPolicy(const String& input_policy); void setInputPanelPolicy(const String& input_policy);
// EventTarget overrides // EventTarget overrides
bool HasEventTargetData() const { return has_event_target_data; }
const AtomicString& InterfaceName() const override; const AtomicString& InterfaceName() const override;
ExecutionContext* GetExecutionContext() const final; ExecutionContext* GetExecutionContext() const override;
// ActiveScriptWrappable overrides.
bool HasPendingActivity() const override;
void Trace(Visitor*) override; void Trace(Visitor*) override;
// WebInputMethodController overrides. // WebInputMethodController overrides.
...@@ -231,7 +235,6 @@ class CORE_EXPORT EditContext final : public EventTargetWithInlineData, ...@@ -231,7 +235,6 @@ class CORE_EXPORT EditContext final : public EventTargetWithInlineData,
ui::TextInputAction enter_key_hint_ = ui::TextInputAction::kEnter; ui::TextInputAction enter_key_hint_ = ui::TextInputAction::kEnter;
EditContextInputPanelPolicy input_panel_policy_ = EditContextInputPanelPolicy input_panel_policy_ =
EditContextInputPanelPolicy::kManual; EditContextInputPanelPolicy::kManual;
bool has_event_target_data = false;
WebRect control_bounds_; WebRect control_bounds_;
WebRect selection_bounds_; WebRect selection_bounds_;
// This flag is set when the input method controller receives a // This flag is set when the input method controller receives a
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
Constructor(optional EditContextInit options), Constructor(optional EditContextInit options),
ConstructorCallWith=ScriptState, ConstructorCallWith=ScriptState,
Exposed=Window, Exposed=Window,
ActiveScriptWrappable,
RuntimeEnabled=EditContext RuntimeEnabled=EditContext
] interface EditContext : EventTarget { ] interface EditContext : EventTarget {
void focus(); void focus();
......
...@@ -105,8 +105,6 @@ crbug.com/1000512 [ Linux ] http/tests/devtools/a11y-axe-core/sources/sources-ed ...@@ -105,8 +105,6 @@ crbug.com/1000512 [ Linux ] http/tests/devtools/a11y-axe-core/sources/sources-ed
# Only times out on the leak bots. # Only times out on the leak bots.
crbug.com/998399 [ Linux ] virtual/omt-worker-fetch/external/wpt/service-workers/service-worker/worker-interception.https.html [ Pass Timeout ] crbug.com/998399 [ Linux ] virtual/omt-worker-fetch/external/wpt/service-workers/service-worker/worker-interception.https.html [ Pass Timeout ]
crbug.com/1023242 [ Linux ] editing/input/edit-context.html [ Pass Failure ]
########################################################################### ###########################################################################
# WARNING: Memory leaks must be fixed asap. Sheriff is expected to revert # # WARNING: Memory leaks must be fixed asap. Sheriff is expected to revert #
# culprit CLs instead of suppressing the leaks. If you have any question, # # culprit CLs instead of suppressing the leaks. If you have any question, #
......
...@@ -457,13 +457,12 @@ test(function() { ...@@ -457,13 +457,12 @@ test(function() {
const textarea = childDocument.createElement('textarea'); const textarea = childDocument.createElement('textarea');
childDocument.body.appendChild(textarea); childDocument.body.appendChild(textarea);
textarea.addEventListener("focusin", e => { textarea.addEventListener("focusin", e => {
childDocument.childEditContext = new EditContext() const childEditContext = new EditContext()
childDocument.childEditContext.focus(); childEditContext.focus();
childDocument.childEditContext.addEventListener("textupdate", e => { childEditContext.addEventListener("textupdate", e => {
console.log("iframe textupdate event fired");
child.remove(); child.remove();
}); });
childDocument.childEditContext.addEventListener("textformatupdate", e => { childEditContext.addEventListener("textformatupdate", e => {
}); });
}); });
textarea.focus(); textarea.focus();
...@@ -472,26 +471,14 @@ test(function() { ...@@ -472,26 +471,14 @@ test(function() {
}, 'Testing EditContext Iframe Document Delete'); }, 'Testing EditContext Iframe Document Delete');
test(function() { test(function() {
// SetComposition should not crash when event handler removes document const editContext1 = new EditContext();
const child = document.createElement("iframe"); editContext1.addEventListener("textupdate", e => {
document.body.appendChild(child);
const childDocument = child.contentDocument;
const textarea = childDocument.createElement('textarea');
childDocument.body.appendChild(textarea);
textarea.addEventListener("focusin", e => {
childDocument.childEditContext = new EditContext()
childDocument.childEditContext.focus()
childDocument.childEditContext.addEventListener("textupdate", e => {
console.log("textupdate event fired");
textarea.remove();
});
childDocument.childEditContext.addEventListener("textformatupdate", e => {
});
}); });
child.contentWindow.focus(); editContext1.focus();
textarea.focus(); gc()
textInputController.setComposition("bar"); textInputController.setComposition("bar");
}, 'Testing EditContext Iframe Element Delete');
}, 'Testing EditContext GC');
</script> </script>
</body> </body>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment