Commit c3957448 authored by David Bokan's avatar David Bokan Committed by Commit Bot

Cleanup and remove dead code in SetFocusedElement

This early-out was added in:

https://crrev.com/ce8ea3446283965c7eabab592cbffe223b1cf2bc

Back then, we applied fragment focus in LayoutUpdated() which could
cause this issue. This got cleaned up in:

https://crrev.com/45236fd563e9df53dc45579be1f3d0b4784885a2

so that focus is no longer applied after layout.

+Cleanup: Goto considered harmful

Bug: 795381
Change-Id: Ifeb4d2e03e872fd48cca6720b1d4de36ad1ecbb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524417
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641101}
parent ded259df
...@@ -4657,8 +4657,10 @@ bool Document::SetFocusedElement(Element* new_focused_element, ...@@ -4657,8 +4657,10 @@ bool Document::SetFocusedElement(Element* new_focused_element,
if (IsRootEditableElement(*new_focused_element) && if (IsRootEditableElement(*new_focused_element) &&
!AcceptsEditingFocus(*new_focused_element)) { !AcceptsEditingFocus(*new_focused_element)) {
// delegate blocks focus change // delegate blocks focus change
focus_change_blocked = true; UpdateStyleAndLayoutTree();
goto SetFocusedElementDone; if (LocalFrame* frame = GetFrame())
frame->Selection().DidChangeFocus();
return false;
} }
// Set focus on the new node // Set focus on the new node
focused_element_ = new_focused_element; focused_element_ = new_focused_element;
...@@ -4673,18 +4675,13 @@ bool Document::SetFocusedElement(Element* new_focused_element, ...@@ -4673,18 +4675,13 @@ bool Document::SetFocusedElement(Element* new_focused_element,
// Element::setFocused for frames can dispatch events. // Element::setFocused for frames can dispatch events.
if (focused_element_ != new_focused_element) { if (focused_element_ != new_focused_element) {
focus_change_blocked = true; UpdateStyleAndLayoutTree();
goto SetFocusedElementDone; if (LocalFrame* frame = GetFrame())
frame->Selection().DidChangeFocus();
return false;
} }
CancelFocusAppearanceUpdate(); CancelFocusAppearanceUpdate();
EnsurePaintLocationDataValidForNode(focused_element_); EnsurePaintLocationDataValidForNode(focused_element_);
// UpdateStyleAndLayout can call SetFocusedElement (through
// InvokeFragmentAnchor called in Document::LayoutUpdated) and clear
// focused_element_.
if (focused_element_ != new_focused_element) {
focus_change_blocked = true;
goto SetFocusedElementDone;
}
focused_element_->UpdateFocusAppearanceWithOptions( focused_element_->UpdateFocusAppearanceWithOptions(
params.selection_behavior, params.options); params.selection_behavior, params.options);
...@@ -4698,8 +4695,10 @@ bool Document::SetFocusedElement(Element* new_focused_element, ...@@ -4698,8 +4695,10 @@ bool Document::SetFocusedElement(Element* new_focused_element,
if (focused_element_ != new_focused_element) { if (focused_element_ != new_focused_element) {
// handler shifted focus // handler shifted focus
focus_change_blocked = true; UpdateStyleAndLayoutTree();
goto SetFocusedElementDone; if (LocalFrame* frame = GetFrame())
frame->Selection().DidChangeFocus();
return false;
} }
// DOM level 3 bubbling focus event. // DOM level 3 bubbling focus event.
focused_element_->DispatchFocusInEvent(event_type_names::kFocusin, focused_element_->DispatchFocusInEvent(event_type_names::kFocusin,
...@@ -4708,8 +4707,10 @@ bool Document::SetFocusedElement(Element* new_focused_element, ...@@ -4708,8 +4707,10 @@ bool Document::SetFocusedElement(Element* new_focused_element,
if (focused_element_ != new_focused_element) { if (focused_element_ != new_focused_element) {
// handler shifted focus // handler shifted focus
focus_change_blocked = true; UpdateStyleAndLayoutTree();
goto SetFocusedElementDone; if (LocalFrame* frame = GetFrame())
frame->Selection().DidChangeFocus();
return false;
} }
// For DOM level 2 compatibility. // For DOM level 2 compatibility.
...@@ -4721,8 +4722,10 @@ bool Document::SetFocusedElement(Element* new_focused_element, ...@@ -4721,8 +4722,10 @@ bool Document::SetFocusedElement(Element* new_focused_element,
if (focused_element_ != new_focused_element) { if (focused_element_ != new_focused_element) {
// handler shifted focus // handler shifted focus
focus_change_blocked = true; UpdateStyleAndLayoutTree();
goto SetFocusedElementDone; if (LocalFrame* frame = GetFrame())
frame->Selection().DidChangeFocus();
return false;
} }
} }
} }
...@@ -4741,7 +4744,6 @@ bool Document::SetFocusedElement(Element* new_focused_element, ...@@ -4741,7 +4744,6 @@ bool Document::SetFocusedElement(Element* new_focused_element,
focused_element_.Get()); focused_element_.Get());
} }
SetFocusedElementDone:
UpdateStyleAndLayoutTree(); UpdateStyleAndLayoutTree();
if (LocalFrame* frame = GetFrame()) if (LocalFrame* frame = GetFrame())
frame->Selection().DidChangeFocus(); frame->Selection().DidChangeFocus();
......
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