Skip to content

Commit

Permalink
Bug 1533293 - part 3: Make editor and ContentEventHandler not use Sel…
Browse files Browse the repository at this point in the history
…ection::Extend() due to too slow r=m_kato

`Selection::Extend()` is too slow but editor and ContentEventHandler use it in
some places.  We should make them use `Selection::SetStartAndEndInLimiter()` or
`Selection::SetBaseAndExtentInLimiter()`.  The former is usable only when caller
guarantees the start point is prior to the end point in the DOM tree.
Otherwise, we need to use the latter even though it's slower than the former.

Differential Revision: https://phabricator.services.mozilla.com/D23462
  • Loading branch information
masayuki-nakano committed Mar 18, 2019
1 parent 6da3046 commit 965ad3e
Show file tree
Hide file tree
Showing 22 changed files with 179 additions and 134 deletions.
40 changes: 18 additions & 22 deletions dom/events/ContentEventHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2981,32 +2981,28 @@ nsresult ContentEventHandler::OnSelectionEvent(WidgetSelectionEvent* aEvent) {
return NS_ERROR_UNEXPECTED;
}

mSelection->StartBatchChanges();

// Clear selection first before setting
rv = mSelection->RemoveAllRangesTemporarily();
// Need to call EndBatchChanges at the end even if call failed
if (NS_SUCCEEDED(rv)) {
if (aEvent->mReversed) {
rv = mSelection->Collapse(endNode, endNodeOffset);
} else {
rv = mSelection->Collapse(startNode, startNodeOffset);
if (aEvent->mReversed) {
nsCOMPtr<nsINode> startNodeStrong(startNode);
nsCOMPtr<nsINode> endNodeStrong(endNode);
ErrorResult error;
MOZ_KnownLive(mSelection)
->SetBaseAndExtentInLimiter(*endNodeStrong, endNodeOffset,
*startNodeStrong, startNodeOffset, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
if (NS_SUCCEEDED(rv) &&
(startNode != endNode || startNodeOffset != endNodeOffset)) {
if (aEvent->mReversed) {
rv = mSelection->Extend(startNode, startNodeOffset);
} else {
rv = mSelection->Extend(endNode, endNodeOffset);
}
} else {
nsCOMPtr<nsINode> startNodeStrong(startNode);
nsCOMPtr<nsINode> endNodeStrong(endNode);
ErrorResult error;
MOZ_KnownLive(mSelection)
->SetBaseAndExtentInLimiter(*startNodeStrong, startNodeOffset,
*endNodeStrong, endNodeOffset, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
}

// Pass the eSetSelection events reason along with the BatchChange-end
// selection change notifications.
mSelection->EndBatchChanges(aEvent->mReason);
NS_ENSURE_SUCCESS(rv, rv);

mSelection->ScrollIntoView(nsISelectionController::SELECTION_FOCUS_REGION,
nsIPresShell::ScrollAxis(),
nsIPresShell::ScrollAxis(), 0);
Expand Down
1 change: 1 addition & 0 deletions dom/events/ContentEventHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class MOZ_STACK_CLASS ContentEventHandler {
nsresult OnQueryDOMWidgetHittest(WidgetQueryContentEvent* aEvent);

// NS_SELECTION_* event
MOZ_CAN_RUN_SCRIPT
nsresult OnSelectionEvent(WidgetSelectionEvent* aEvent);

protected:
Expand Down
6 changes: 4 additions & 2 deletions dom/events/EventStateManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -750,10 +750,12 @@ nsresult EventStateManager::PreHandleEvent(nsPresContext* aPresContext,
DeltaAccumulator::GetInstance()->InitLineOrPageDelta(aTargetFrame, this,
wheelEvent);
} break;
case eSetSelection:
IMEStateManager::HandleSelectionEvent(aPresContext, GetFocusedContent(),
case eSetSelection: {
nsCOMPtr<nsIContent> focusedContent = GetFocusedContent();
IMEStateManager::HandleSelectionEvent(aPresContext, focusedContent,
aEvent->AsSelectionEvent());
break;
}
case eContentCommandCut:
case eContentCommandCopy:
case eContentCommandPaste:
Expand Down
1 change: 1 addition & 0 deletions dom/events/IMEStateManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ class IMEStateManager {
* because they must be handled by same target as composition events when
* there is a composition.
*/
MOZ_CAN_RUN_SCRIPT
static void HandleSelectionEvent(nsPresContext* aPresContext,
nsIContent* aEventTargetContent,
WidgetSelectionEvent* aSelectionEvent);
Expand Down
6 changes: 5 additions & 1 deletion dom/events/TextComposition.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,9 +456,13 @@ class TextComposition final {
* HandleSelectionEvent() sends the selection event to ContentEventHandler
* or dispatches it to the focused child process.
*/
MOZ_CAN_RUN_SCRIPT
void HandleSelectionEvent(WidgetSelectionEvent* aSelectionEvent) {
HandleSelectionEvent(mPresContext, mTabParent, aSelectionEvent);
RefPtr<nsPresContext> presContext(mPresContext);
RefPtr<TabParent> tabParent(mTabParent);
HandleSelectionEvent(presContext, tabParent, aSelectionEvent);
}
MOZ_CAN_RUN_SCRIPT
static void HandleSelectionEvent(nsPresContext* aPresContext,
TabParent* aTabParent,
WidgetSelectionEvent* aSelectionEvent);
Expand Down
1 change: 1 addition & 0 deletions editor/libeditor/EditorBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ class EditorBase : public nsIEditor,
return mTransactionManager->RemoveTransactionListener(aListener);
}

MOZ_CAN_RUN_SCRIPT
virtual nsresult HandleKeyPressEvent(WidgetKeyboardEvent* aKeyboardEvent);

virtual dom::EventTarget* GetDOMEventTarget() = 0;
Expand Down
65 changes: 26 additions & 39 deletions editor/libeditor/HTMLEditRules.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1731,7 +1731,8 @@ EditActionResult HTMLEditRules::WillInsertParagraphSeparator() {
// MakeBasicBlock() creates AutoSelectionRestorer.
// Therefore, even if it returns NS_OK, editor might have been destroyed
// at restoring Selection.
nsresult rv = MakeBasicBlock(ParagraphSeparatorElement(separator));
OwningNonNull<nsAtom> separatorTag = ParagraphSeparatorElement(separator);
nsresult rv = MakeBasicBlock(separatorTag);
if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED) ||
NS_WARN_IF(!CanHandleEditAction())) {
return EditActionIgnored(NS_ERROR_EDITOR_DESTROYED);
Expand Down Expand Up @@ -2221,7 +2222,8 @@ nsresult HTMLEditRules::WillDeleteSelection(
HTMLEditorRef().GetFirstSelectedTableCellElement(error);
if (cellElement) {
error.SuppressException();
nsresult rv = HTMLEditorRef().DeleteTableCellContentsWithTransaction();
nsresult rv =
MOZ_KnownLive(HTMLEditorRef()).DeleteTableCellContentsWithTransaction();
if (NS_WARN_IF(!CanHandleEditAction())) {
return NS_ERROR_EDITOR_DESTROYED;
}
Expand Down Expand Up @@ -6508,13 +6510,6 @@ nsresult HTMLEditRules::ExpandSelectionForDeletion() {
}
}
}
// Now set the selection to the new range
DebugOnly<nsresult> rv =
SelectionRefPtr()->Collapse(selStartNode, selStartOffset);
if (NS_WARN_IF(!CanHandleEditAction())) {
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to collapse selection");

// Expand selection endpoint only if we didn't pass a <br>, or if we really
// needed to pass that <br> (i.e., its block is now totally selected).
Expand Down Expand Up @@ -6542,26 +6537,20 @@ nsresult HTMLEditRules::ExpandSelectionForDeletion() {
doEndExpansion = false;
}
}
if (doEndExpansion) {
nsresult rv = SelectionRefPtr()->Extend(selEndNode, selEndOffset);
if (NS_WARN_IF(!CanHandleEditAction())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
} else {
// Only expand to just before <br>.
nsresult rv = SelectionRefPtr()->Extend(firstBRParent, firstBROffset);
if (NS_WARN_IF(!CanHandleEditAction())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
}

return NS_OK;
EditorRawDOMPoint newSelectionStart(selStartNode, selStartOffset);
EditorRawDOMPoint newSelectionEnd(
doEndExpansion ? selEndNode : firstBRParent,
doEndExpansion ? selEndOffset : firstBROffset);
ErrorResult error;
MOZ_KnownLive(SelectionRefPtr())
->SetStartAndEndInLimiter(newSelectionStart, newSelectionEnd, error);
if (NS_WARN_IF(!CanHandleEditAction())) {
error.SuppressException();
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(!error.Failed(), "Failed to set selection for deletion");
return error.StealNSResult();
}

nsresult HTMLEditRules::NormalizeSelection() {
Expand Down Expand Up @@ -6714,20 +6703,18 @@ nsresult HTMLEditRules::NormalizeSelection() {
return NS_OK; // New start after old end.
}

// otherwise set selection to new values.
// XXX Why don't we use SetBaseAndExtent()?
DebugOnly<nsresult> rv =
SelectionRefPtr()->Collapse(newStartNode, newStartOffset);
if (NS_WARN_IF(!CanHandleEditAction())) {
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to collapse selection");
rv = SelectionRefPtr()->Extend(newEndNode, newEndOffset);
// Otherwise set selection to new values. Note that end point may be prior
// to start point. So, we cannot use Selection::SetStartAndEndInLimit() here.
ErrorResult error;
MOZ_KnownLive(SelectionRefPtr())
->SetBaseAndExtentInLimiter(*newStartNode, newStartOffset, *newEndNode,
newEndOffset, error);
if (NS_WARN_IF(!CanHandleEditAction())) {
error.SuppressException();
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to extend selection");
return NS_OK;
NS_WARNING_ASSERTION(!error.Failed(), "Failed to set selection");
return error.StealNSResult();
}

EditorDOMPoint HTMLEditRules::GetPromotedPoint(RulesEndpoint aWhere,
Expand Down
14 changes: 14 additions & 0 deletions editor/libeditor/HTMLEditRules.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillDeleteSelection(
nsIEditor::EDirection aAction, nsIEditor::EStripWrappers aStripWrappers,
bool* aCancel, bool* aHandled);
Expand Down Expand Up @@ -374,6 +375,7 @@ class HTMLEditRules : public TextEditRules {
/**
* XXX Should document what this does.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillMakeList(const nsAString* aListType,
bool aEntireList,
const nsAString* aBulletType,
Expand All @@ -388,6 +390,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillRemoveList(bool* aCancel, bool* aHandled);

/**
Expand All @@ -397,6 +400,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillIndent(bool* aCancel, bool* aHandled);

/**
Expand All @@ -406,6 +410,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillCSSIndent(bool* aCancel, bool* aHandled);

/**
Expand All @@ -415,6 +420,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillHTMLIndent(bool* aCancel, bool* aHandled);

/**
Expand All @@ -424,6 +430,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillOutdent(bool* aCancel, bool* aHandled);

/**
Expand All @@ -435,6 +442,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
nsresult WillAlign(const nsAString& aAlignType, bool* aCancel,
bool* aHandled);

Expand Down Expand Up @@ -473,6 +481,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillMakeDefListItem(const nsAString* aBlockType,
bool aEntireList, bool* aCancel,
bool* aHandled);
Expand All @@ -485,6 +494,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillMakeBasicBlock(const nsAString& aBlockType,
bool* aCancel, bool* aHandled);

Expand All @@ -501,6 +511,7 @@ class HTMLEditRules : public TextEditRules {
* will be called.
* Otherwise, ApplyBlockStyle() will be called.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult MakeBasicBlock(nsAtom& aBlockType);

/**
Expand All @@ -519,6 +530,7 @@ class HTMLEditRules : public TextEditRules {
* @param aCancel Returns true if the operation is canceled.
* @param aHandled Returns true if the edit action is handled.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult WillAbsolutePosition(bool* aCancel, bool* aHandled);

/**
Expand Down Expand Up @@ -848,6 +860,7 @@ class HTMLEditRules : public TextEditRules {
* invisible <br> element for preventing delete action handler to keep
* unexpected nodes.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult ExpandSelectionForDeletion();

/**
Expand All @@ -856,6 +869,7 @@ class HTMLEditRules : public TextEditRules {
* non-editable point, they should be moved to nearest text node or something
* where the other methods easier to handle edit action.
*/
MOZ_CAN_RUN_SCRIPT
MOZ_MUST_USE nsresult NormalizeSelection();

/**
Expand Down
32 changes: 11 additions & 21 deletions editor/libeditor/HTMLEditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1727,7 +1727,7 @@ HTMLEditor::SelectElement(Element* aElement) {
return NS_ERROR_NOT_INITIALIZED;
}

nsresult rv = SelectContentInternal(*aElement);
nsresult rv = SelectContentInternal(MOZ_KnownLive(*aElement));
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
Expand All @@ -1742,28 +1742,18 @@ nsresult HTMLEditor::SelectContentInternal(nsIContent& aContentToSelect) {
return NS_ERROR_FAILURE;
}

nsINode* parent = aContentToSelect.GetParentNode();
if (NS_WARN_IF(!parent)) {
EditorRawDOMPoint newSelectionStart(&aContentToSelect);
if (NS_WARN_IF(!newSelectionStart.IsSet())) {
return NS_ERROR_FAILURE;
}

// Don't notify selection change at collapse.
AutoUpdateViewBatch notifySelectionChangeOnce(*this);

// XXX Perhaps, Selection should have SelectNode(nsIContent&).
int32_t offsetInParent = parent->ComputeIndexOf(&aContentToSelect);

// Collapse selection to just before desired element,
nsresult rv = SelectionRefPtr()->Collapse(parent, offsetInParent);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
// then extend it to just after
rv = SelectionRefPtr()->Extend(parent, offsetInParent + 1);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
EditorRawDOMPoint newSelectionEnd(&aContentToSelect);
MOZ_ASSERT(newSelectionEnd.IsSet());
DebugOnly<bool> advanced = newSelectionEnd.AdvanceOffset();
ErrorResult error;
MOZ_KnownLive(SelectionRefPtr())
->SetStartAndEndInLimiter(newSelectionStart, newSelectionEnd, error);
NS_WARNING_ASSERTION(!error.Failed(), "Failed to select the given content");
return error.StealNSResult();
}

NS_IMETHODIMP
Expand Down
Loading

0 comments on commit 965ad3e

Please sign in to comment.