This is just the typo of the condition, but this have made a lot of
queries failed when there is no IME selections. This could be one of
the cause of bug 1824143.
Differential Revision: https://phabricator.services.mozilla.com/D272821
This reverts commit 0062ee7423.
Revert "Bug 1983375 - part 1: Define `MOZ_LOG_ONLY_DEBUG` and variants in `Logging.h` and sort it out `GetBoolName` for making it more generic r=emilio,padenot"
This reverts commit b61fa4819b.
When I make `ContentCacheInParent` manage the pending content command events, I forgot
to make `BrowserChild::RecvInsertText()` and `BrowserChild::RecvReplaceText()` call
`SendOnEventNeedingAckHandled()` because they have existed and I added
`BrowserChild::RecvSimpleContentCommandEvent()`. Therefore, this patch makes them call
`SendOnEventNeedingAckHandled()`.
Additionally, `ContentCacheInParent::OnEventNeedingAckHandled()` returns without
managing the count if it's called for the content command events. Therefore, this patch
fixes the early return bug too.
Differential Revision: https://phabricator.services.mozilla.com/D225749
Currently, `EventStateManager` handles some content commands only in the parent
process. Therefore, if `eSetSelection` event or something is being handled in
the focused remote process, the command may be disabled yet.
Avoiding the race conditions, we should handle the content command events in the
focused process.
Differential Revision: https://phabricator.services.mozilla.com/D222789
Currently, `EventStateManager` handles some content commands only in the parent
process. Therefore, if `eSetSelection` event or something is being handled in
the focused remote process, the command may be disabled yet.
Avoiding the race conditions, we should handle the content command events in the
focused process.
Differential Revision: https://phabricator.services.mozilla.com/D222789
The assertion reason "MOZ_DIAGNOSTIC_ASSERT(IsValid())" does not help us to
debug. Make it assert with the minimum data of `ContentCache` instead.
Depends on D182652
Differential Revision: https://phabricator.services.mozilla.com/D182653
Once `PuppetWidget` sends invalid data, the parent process will return error
and that makes the sender content process crashed. Therefore, if we detect
that we created invalid data, we should not send the invalid data to avoid
the crash.
Differential Revision: https://phabricator.services.mozilla.com/D182652
E.g., when a character is inserted with a composition, i.e., without composing
state, like the Emoji picker of Windows, `BrowserParent` dispatches
`eCompositionStart`, `eCompositionChange` and `eCompositionCommit` message
within a short time. Then, if focus is moved at `compositionstart` event in
the remote message, `eCompositionChange` and `eCompositionCommit` are not yet
handled by the remote process when `BrowserParent::OnEventNeedingAckHandled()`
received a commit request handled message. Therefore, it's not invalid case
that a commit is handled without acknowledges of multiple pending composition
messages. Therefore, the assertion should be removed, and instead, for making
it easier to understand the log, let's put the information of that. Then, it's
obvious that ignored log of following `OnEventNeedingAckHandled()` calls is
valid thing.
Differential Revision: https://phabricator.services.mozilla.com/D181217
Then, we can know whether the message notification and commit request is for
proper one or outdated one, etc. Therefore, this patch makes
`OnEventNeedingAckHandled()` and `RequestIMEToCommitComposition()` ignores
unexpected and/or outdated calls. This should fix the crash reported in
bug 1466596 too.
Differential Revision: https://phabricator.services.mozilla.com/D179313
It's now not necessary. We can check all pending composition data whether
there are some composition which waits commit in the remote process.
Differential Revision: https://phabricator.services.mozilla.com/D179312
We need to make `ContentCacheInParent` store each pending composition data
for ignoring unexpected (including outdated) composition request/notifications
for protecting the main process's data, e.g., current composition.
This patch creates the place (array) and just simply store whether the
pending composition has already sent `eCompositionCommit(AsIs)` to the
remote process.
Differential Revision: https://phabricator.services.mozilla.com/D179311
For handling (ignoring) "too late" composition commit request from content
process, we need to distinguish a request is for which composition. Therefore,
we need to number each composition originated in the parent process.
This patch makes `TextComposition` instance which is created at first
composition event for a composition consider a composition ID in the parent
process and set it to composition events which are dispatched into the DOM
tree in the parent or sent to a remote process.
And also this patch adds the composition ID param to the request method of
committing composition and reply methods to notify the parent process of ending
a composition event handling.
The last patch handle them in `ContentCacheInParent` to consider
whether a request/reply should be ignored or handled.
Differential Revision: https://phabricator.services.mozilla.com/D179310
Then, we can know whether the message notification and commit request is for
proper one or outdated one, etc. Therefore, this patch makes
`OnEventNeedingAckHandled()` and `RequestIMEToCommitComposition()` ignores
unexpected and/or outdated calls. This should fix the crash reported in
bug 1466596 too.
Differential Revision: https://phabricator.services.mozilla.com/D179313
It's now not necessary. We can check all pending composition data whether
there are some composition which waits commit in the remote process.
Differential Revision: https://phabricator.services.mozilla.com/D179312
We need to make `ContentCacheInParent` store each pending composition data
for ignoring unexpected (including outdated) composition request/notifications
for protecting the main process's data, e.g., current composition.
This patch creates the place (array) and just simply store whether the
pending composition has already sent `eCompositionCommit(AsIs)` to the
remote process.
Differential Revision: https://phabricator.services.mozilla.com/D179311
For handling (ignoring) "too late" composition commit request from content
process, we need to distinguish a request is for which composition. Therefore,
we need to number each composition originated in the parent process.
This patch makes `TextComposition` instance which is created at first
composition event for a composition consider a composition ID in the parent
process and set it to composition events which are dispatched into the DOM
tree in the parent or sent to a remote process.
And also this patch adds the composition ID param to the request method of
committing composition and reply methods to notify the parent process of ending
a composition event handling.
The last patch handle them in `ContentCacheInParent` to consider
whether a request/reply should be ignored or handled.
Differential Revision: https://phabricator.services.mozilla.com/D179310
It creates no range `Selection` if `mSelection` has not been emplaced.
However, we don't want `Selection` in the case because `mText` may be nothing
and that violates the dependency. Therefore, it should stop creating
`Selection` in the case.
Note that first character rect will be cached later even if there is no
`Selection`. However, this should not occur in usual case because `focus`
notification should've already initialized `mText` and `mSelection`.
Differential Revision: https://phabricator.services.mozilla.com/D178731
Currently, `PuppetWidget` calls `ContentCacheInChild::CacheSelection` directly.
However, `mText` can be `Nothing` or `mText` can be outdated, but `mSelection`
becomes the latest one. Therefore, we may notify the parent process with
invalid data combination.
The callers in `PuppetWidget` are:
1. `NotifyIMEOfCompositionUpdate`
2. `NotifyIMEOfPositionChange`
I think that the former does not need to cache anything here because
`IMEContentObserver` should've updated the text/selection changes. However,
stopping caching everything at this point is risky. In the most cases, outdated
data appears as odd IME UI position. Therefore, let's keep updating only caret
and text rectangles.
The latter is reported with new crash reports which is crashed by a
`MOZ_DIAGNOSTIC_ASSERT` failure added by the previous patch. In the case,
if `mText` and `mSelection` has not been cached, we don't need to notify
the parent process with them because they will be sent later. And also even
if they are not available, it may be useful that the character rectangles not
related to `Selection` (e.g., the first character rect). Therefore, let's
keep caching same things as the former case.
Therefore, this patch makes `CacheSelection` a private method and add
`CacheCaretAndTextRects` for them.
Differential Revision: https://phabricator.services.mozilla.com/D178145
There are some crash reports crashed in TSF module which may be caused by
passing invalid selection range (e.g., out of bounds of text). However,
the cache is created in the child process and that causes the invalid cache
creation does not appear in the crash reports. Therefore, let's try to
crash as soon as possible if `ContentCache` has invalid data.
Note that this does not detect all of the invalid cases because it's hard to
(re-)understand the edge cases. Therefore, this tries to detect the cases
checked in `ContentCacheInParent::HandleQueryContentEvent` (*1) and some other
obvious odd cases.
1. https://searchfox.org/mozilla-central/rev/0ffaecaa075887ab07bf4c607c61ea2faa81b172/widget/ContentCache.cpp#776-778
Differential Revision: https://phabricator.services.mozilla.com/D176747
Currently, `PuppetWidget` calls `ContentCacheInChild::CacheSelection` directly.
However, `mText` can be `Nothing` or `mText` can be outdated, but `mSelection`
becomes the latest one. Therefore, we may notify the parent process with
invalid data combination.
The callers in `PuppetWidget` are:
1. `NotifyIMEOfCompositionUpdate`
2. `NotifyIMEOfPositionChange`
I think that the former does not need to cache anything here because
`IMEContentObserver` should've updated the text/selection changes. However,
stopping caching everything at this point is risky. In the most cases, outdated
data appears as odd IME UI position. Therefore, let's keep updating only caret
and text rectangles.
The latter is reported with new crash reports which is crashed by a
`MOZ_DIAGNOSTIC_ASSERT` failure added by the previous patch. In the case,
if `mText` and `mSelection` has not been cached, we don't need to notify
the parent process with them because they will be sent later. And also even
if they are not available, it may be useful that the character rectangles not
related to `Selection` (e.g., the first character rect). Therefore, let's
keep caching same things as the former case.
Therefore, this patch makes `CacheSelection` a private method and add
`CacheCaretAndTextRects` for them.
Depends on D176747
Differential Revision: https://phabricator.services.mozilla.com/D178145
There are some crash reports crashed in TSF module which may be caused by
passing invalid selection range (e.g., out of bounds of text). However,
the cache is created in the child process and that causes the invalid cache
creation does not appear in the crash reports. Therefore, let's try to
crash as soon as possible if `ContentCache` has invalid data.
Note that this does not detect all of the invalid cases because it's hard to
(re-)understand the edge cases. Therefore, this tries to detect the cases
checked in `ContentCacheInParent::HandleQueryContentEvent` (*1) and some other
obvious odd cases.
1. https://searchfox.org/mozilla-central/rev/0ffaecaa075887ab07bf4c607c61ea2faa81b172/widget/ContentCache.cpp#776-778
Differential Revision: https://phabricator.services.mozilla.com/D176747
There are some crash reports crashed in TSF module which may be caused by
passing invalid selection range (e.g., out of bounds of text). However,
the cache is created in the child process and that causes the invalid cache
creation does not appear in the crash reports. Therefore, let's try to
crash as soon as possible if `ContentCache` has invalid data.
Note that this does not detect all of the invalid cases because it's hard to
(re-)understand the edge cases. Therefore, this tries to detect the cases
checked in `ContentCacheInParent::HandleQueryContentEvent` (*1) and some other
obvious odd cases.
1. https://searchfox.org/mozilla-central/rev/0ffaecaa075887ab07bf4c607c61ea2faa81b172/widget/ContentCache.cpp#776-778
Differential Revision: https://phabricator.services.mozilla.com/D176747
It's designed for caching content information of focused editor. However, at
sending focus notification to the main process, the editor may have already
been blurred but `IMEContentObserver` may have not known it yet. In this edge
case, only query selection succeeds since IMEContentObserver still has a cache,
but query the others failed because of root content node check of
`IMEContentObserver::HandleQueryContentEvent`. If a content process meets this
case, it should not send focus notification and stop storing the content since
IME shouldn't get focus nor query non-editable content.
On the other hand, the reported testcase reproduces this with a fuzzing API
called **in** the content process. Therefore, I have no idea how to reproduce
it without the API. That's the reason why this patch does not contain new tests.
Differential Revision: https://phabricator.services.mozilla.com/D141821
Unfortunately, the adding test cannot reproduce the crash, but I have no idea
how to make `mSelection` being `Nothing` until next native event loop. Anyway,
it's worthwhile to add new tests.
This patch just avoids `ContentCacheInParent::GetTextRect` referring
`mSelection` when there is no text rects and `mSelection` is `Nothing`.
Differential Revision: https://phabricator.services.mozilla.com/D138876
Oddly, in some complicated web apps, `IMEContentObserver` may get selection
change notifications **after** sending a text change notification to IME.
However, I cannot reproduce it with simple testcase. Therefore, the new test
does not test the original issue, but the approach of this patch must avoid
regressions of this special case handling.
Differential Revision: https://phabricator.services.mozilla.com/D137522
Currently, `mSelection` is cached only when `mText` is some, text rects are
cached only when `mSelection` is cached. However, as far as widget can work
with IME, it should cache content as far as possible. Therefore, this patch
makes it keep querying content data even after something fails.
Differential Revision: https://phabricator.services.mozilla.com/D137431
Even after applying this patch, it still returns error when queried with a
relative offset from selection and only when there is no composition string.
However, this shouldn't cause any problem because in this case, widget should
not try to query content with relative offset.
Differential Revision: https://phabricator.services.mozilla.com/D137430
Masayuki Nakano