Bug 1737919 part 1: TextLeafPoint::FindTextAttrsStart: Don't allow the caller to supply origin attributes. r=morgan

This was done for LocalAccessibles to avoid the need to fetch attributes twice when we're simultaneously fetching boundaries and returning the attributes to a client.
However, GetTextAttributes will soon handle spelling errors, so it will no longer return the same data as GetTextAttributesLocalAcc (which doesn't handle spelling errors).
This breaks the equality checks in FindTextAttrsStart.
Aside from this, this argument was somewhat confusing.

For now, just remove aOriginAttrs, which means there will be some redundant calls to GetTextAttributesLocalAcc.
Parent process documents shouldn't be that large anyway.
If this ends up being a real problem, we can either revert to the local HyperTextAccessible implementation of text attributes or implement a smarter temporary cache.

Differential Revision: https://phabricator.services.mozilla.com/D147241
This commit is contained in:
James Teh 2022-05-27 10:56:40 +00:00
Родитель 65a2647e01
Коммит efe397519e
3 изменённых файлов: 14 добавлений и 45 удалений

Просмотреть файл

@ -1061,36 +1061,15 @@ already_AddRefed<AccAttributes> TextLeafPoint::GetTextAttributes(
return attrs.forget();
}
TextLeafPoint TextLeafPoint::FindTextAttrsStart(
nsDirection aDirection, bool aIncludeOrigin,
const AccAttributes* aOriginAttrs, bool aIncludeDefaults) const {
TextLeafPoint TextLeafPoint::FindTextAttrsStart(nsDirection aDirection,
bool aIncludeOrigin) const {
if (IsCaret()) {
return ActualizeCaret().FindTextAttrsStart(aDirection, aIncludeOrigin,
aOriginAttrs, aIncludeDefaults);
return ActualizeCaret().FindTextAttrsStart(aDirection, aIncludeOrigin);
}
// XXX Add support for spelling errors.
RefPtr<const AccAttributes> lastAttrs;
const bool isRemote = mAcc->IsRemote();
if (isRemote) {
// For RemoteAccessible, leaf attrs and default attrs are cached
// separately. To combine them, we have to copy. Since we're not walking
// outside the container, we don't care about defaults. Therefore, we
// always just fetch the leaf attrs.
// We ignore aOriginAttrs because it might include defaults. Fetching leaf
// attrs is very cheap anyway.
lastAttrs = mAcc->AsRemote()->GetCachedTextAttributes();
} else {
// For LocalAccessible, we want to avoid calculating attrs more than
// necessary, so we want to use aOriginAttrs if provided.
if (aOriginAttrs) {
lastAttrs = aOriginAttrs;
// Whether we include defaults henceforth must match aOriginAttrs, which
// depends on aIncludeDefaults. Defaults are always calculated even if
// they aren't returned, so calculation cost isn't a concern.
} else {
lastAttrs = GetTextAttributesLocalAcc(aIncludeDefaults);
}
}
RefPtr<const AccAttributes> lastAttrs =
isRemote ? mAcc->AsRemote()->GetCachedTextAttributes()
: GetTextAttributesLocalAcc();
if (aIncludeOrigin && aDirection == eDirNext && mOffset == 0) {
// Even when searching forward, the only way to know whether the origin is
// the start of a text attrs run is to compare with the previous sibling.
@ -1104,7 +1083,7 @@ TextLeafPoint TextLeafPoint::FindTextAttrsStart(
// calculation or copying.
RefPtr<const AccAttributes> attrs =
isRemote ? point.mAcc->AsRemote()->GetCachedTextAttributes()
: point.GetTextAttributesLocalAcc(aIncludeDefaults);
: point.GetTextAttributesLocalAcc();
if (attrs && lastAttrs && !attrs->Equal(lastAttrs)) {
return *this;
}
@ -1119,7 +1098,7 @@ TextLeafPoint TextLeafPoint::FindTextAttrsStart(
}
RefPtr<const AccAttributes> attrs =
isRemote ? point.mAcc->AsRemote()->GetCachedTextAttributes()
: point.GetTextAttributesLocalAcc(aIncludeDefaults);
: point.GetTextAttributesLocalAcc();
if (attrs && lastAttrs && !attrs->Equal(lastAttrs)) {
// The attributes change here. If we're moving forward, we want to
// return this point. If we're moving backward, we've now moved before

Просмотреть файл

@ -135,18 +135,9 @@ class TextLeafPoint final {
* (depending on the direction).
* If aIncludeorigin is true and this is at a boundary, this will be
* returned unchanged.
* aOriginAttrs allows the caller to supply the text attributes for this (as
* retrieved by GetTextAttributes). This can be used to avoid fetching the
* attributes twice if they are also to be used for something else; e.g.
* returning the attributes to a client. If aOriginAttrs is null, this method
* will fetch the attributes itself.
* aIncludeDefaults specifies whether aOriginAttrs includes default
* attributes.
*/
TextLeafPoint FindTextAttrsStart(nsDirection aDirection,
bool aIncludeOrigin = false,
const AccAttributes* aOriginAttrs = nullptr,
bool aIncludeDefaults = true) const;
bool aIncludeOrigin = false) const;
/**
* Returns a rect (in dev pixels) describing position and size of

Просмотреть файл

@ -616,17 +616,16 @@ already_AddRefed<AccAttributes> HyperTextAccessibleBase::TextAttributes(
}
TextLeafPoint origin = ToTextLeafPoint(static_cast<int32_t>(offset));
RefPtr<AccAttributes> attributes = origin.GetTextAttributes(aIncludeDefAttrs);
TextLeafPoint start = origin.FindTextAttrsStart(
eDirPrevious, /* aIncludeOrigin */ true, attributes, aIncludeDefAttrs);
TextLeafPoint start =
origin.FindTextAttrsStart(eDirPrevious, /* aIncludeOrigin */ true);
bool ok;
std::tie(ok, *aStartOffset) = TransformOffset(start.mAcc, start.mOffset,
/* aIsEndOffset */ false);
TextLeafPoint end = origin.FindTextAttrsStart(
eDirNext, /* aIncludeOrigin */ false, attributes, aIncludeDefAttrs);
TextLeafPoint end =
origin.FindTextAttrsStart(eDirNext, /* aIncludeOrigin */ false);
std::tie(ok, *aEndOffset) = TransformOffset(end.mAcc, end.mOffset,
/* aIsEndOffset */ true);
return attributes.forget();
return origin.GetTextAttributes(aIncludeDefAttrs);
}
void HyperTextAccessibleBase::CroppedSelectionRanges(