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
It's intended to indicate whether the selection is collapsed or not, but it can
be referred by other members, there is no reasonable user and the name makes
developers confused.
Differential Revision: https://phabricator.services.mozilla.com/D137422
Sorry for this big patch.
This makes `WidgetQueryContentEvent::Reply` is stored with `Maybe` to get
rid of `WidgetQueryContentEvent`. And `Reply` stores offset and string
with `Maybe` and ``OffsetAndData<uint32_t>`, and also tentative caret offset
with `Maybe`. Then, we can get rid of `WidgetQueryContentEvent::NOT_FOUND`.
Note that I tried to make `OffsetAndData` have a method to create `NSRange`
for cocoa widget. However, it causes the column limit`to 100 or longer
and that causes unrelated changes in `TextEvents.h` and `IMEData.h`.
Therefore, I create an inline function in `TextInputHandler.mm` instead.
Differential Revision: https://phabricator.services.mozilla.com/D98264
Usually, IME sets selection and considers candidate list position at starting
new composition. However, Apple Japanese IME sometimes consider the candidate
list position at retrieving the character rects before setting selection.
Therefore, we need to store last commit string's character rects, but don't
need to store it in long time because Kakutei-Undo is supported by Japanese
IMEs and they work only immediately after committing a composition. E.g.,
after moving caret, it won't be available.
Depends on D97838
Differential Revision: https://phabricator.services.mozilla.com/D97839
Currently, it manages the composition start offset with `uint32_t` and setting
it to `UINT32_MAX` when there is no composition. But this is now rewritable
with `Maybe<uint32_t>` for easier to read.
Differential Revision: https://phabricator.services.mozilla.com/D97838
Immediately after committing composition, i.e., still remote content is
handling the commit, `ContentCacheInParent` does not think that it still
has composition, but `TSFTextStore::GetTextExt()` tries a query whose offset
is relative offset from the last composition start offset and then,
`ContentCacheInParent` solves it with selection start (typically, the last
composition end offset). Therefore, this may cause returning error from
`GetTextExt()` and some TIP may fail to do something for next typing.
This patch makes `TSFTextStore::GetTextExt()` consider whether it'll query
content with relative offset from last composition start or selection start,
from `TextEventDispatcher::IsComposing()` result rather than
`TextEventDispatcher::IsHandlingComposition()` since the former means whether
there is composition in the chrome process, but the latter is there is
composition in focused process, and `ContentCacheInParent` state matches the
former.
Depends on D97270
Differential Revision: https://phabricator.services.mozilla.com/D97392
This was done by:
This was done by applying:
```
diff --git a/python/mozbuild/mozbuild/code-analysis/mach_commands.py b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
index 789affde7bbf..fe33c4c7d4d1 100644
--- a/python/mozbuild/mozbuild/code-analysis/mach_commands.py
+++ b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
@@ -2007,7 +2007,7 @@ class StaticAnalysis(MachCommandBase):
from subprocess import Popen, PIPE, check_output, CalledProcessError
diff_process = Popen(self._get_clang_format_diff_command(commit), stdout=PIPE)
- args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format]
+ args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format, '-sort-includes']
if not output_file:
args.append("-i")
```
Then running `./mach clang-format -c <commit-hash>`
Then undoing that patch.
Then running check_spidermonkey_style.py --fixup
Then running `./mach clang-format`
I had to fix four things:
* I needed to move <utility> back down in GuardObjects.h because I was hitting
obscure problems with our system include wrappers like this:
0:03.94 /usr/include/stdlib.h:550:14: error: exception specification in declaration does not match previous declaration
0:03.94 extern void *realloc (void *__ptr, size_t __size)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/malloc_decls.h:53:1: note: previous declaration is here
0:03.94 MALLOC_DECL(realloc, void*, void*, size_t)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozilla/mozalloc.h:22:32: note: expanded from macro 'MALLOC_DECL'
0:03.94 MOZ_MEMORY_API return_type name##_impl(__VA_ARGS__);
0:03.94 ^
0:03.94 <scratch space>:178:1: note: expanded from here
0:03.94 realloc_impl
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozmemory_wrap.h:142:41: note: expanded from macro 'realloc_impl'
0:03.94 #define realloc_impl mozmem_malloc_impl(realloc)
Which I really didn't feel like digging into.
* I had to restore the order of TrustOverrideUtils.h and related files in nss
because the .inc files depend on TrustOverrideUtils.h being included earlier.
* I had to add a missing include to RollingNumber.h
* Also had to partially restore include order in JsepSessionImpl.cpp to avoid
some -WError issues due to some static inline functions being defined in a
header but not used in the rest of the compilation unit.
Differential Revision: https://phabricator.services.mozilla.com/D60327
--HG--
extra : moz-landing-system : lando
rg -l 'mozilla/Move.h' | xargs sed -i 's/#include "mozilla\/Move.h"/#include <utility>/g'
Further manual fixups and cleanups to the include order incoming.
Differential Revision: https://phabricator.services.mozilla.com/D60323
--HG--
extra : moz-landing-system : lando
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
Currently, ContentCacheInParent uses selection when it handles query content
event whose input offset is relative one after sending eCompositionCommit(AsIs)
event but it's not yet handled by the remote process. However, in this case,
selection may not be modified with committed string.
So, when mPendingCommitCount is not 0, ContentCacheInParent should compute
absolute offset with the latest composition string information. For doing
this, it needs to keep storing mCompositionStart until eCompositionCommit(AsIs)
is handled in the remote process actually.
MozReview-Commit-ID: 2Dc69HNIbvh
--HG--
extra : rebase_source : 4be432ad363022e4b3f2e3c82c8d229dc9af889d
After fixing bug 1420849, remote process started to ignore composition events
after it synthesizes eCompositionCommit event after requesting to commit
composition. However, remote process still keeps returning composition events
when it receives from the main process. So, if the main process has already
sent eCompositionCommit(AsIs) event when it's requested to commit composition
from the remote process, ContentCacheInParent::OnEventNeedingAckHandled()
receives both eCompositionCommitRequestHandled and eCompositionCommit(AsIs)
events for *a* composition. Therefore, mPendingCompositionCount may be
decremented twice for a composition. This causes hitting MOZ_DIAGNOSTIC_ASSERT.
So, ContentCacheInParent need to manage if sent composition events are ignored
or not. Then, ContentCacheInParent::OnEventNeedingAckHandled() stops
decrementing mPendingCompositionCount when it receives eCompositionCommit(AsIs)
events which are ignored by the remote process.
This patch manages it with |mIsChildIgnoringCompositionEvents| and changes
|bool mIsPendingLastCommitEvent| to |uint8_t mPendingCommitCount| for
making ContentCache be able to manage multiple pending commit events if
its remote process is too busy.
MozReview-Commit-ID: CYQDeZXl7TJ
--HG--
extra : rebase_source : 6de1e2f1302d556d45d19c73b4d1ea3f86b65373
When ContentCacheInParent::RequestIMEToCommitComposition() returns true,
the remote process will synthesize eCompositionCommit event. This causes
destroying TextComposition event in the remote process (by
IMEStateManager::DispatchCompositionEvent()). Then,
IMEStateManager::DispatchCompositionEvent() will recreate new TextComposition
instance when it receives new composition event. Then, it may request
to commit composition to the main process via PuppetWidget accidentally.
So, after PuppetWidget::RequestIMEToCommitComposition() synthesizes
eCompositionCommit, PuppetWidget should discard following composition events
until it receives eCompositionStart since they are unnecessary for the content
and they are for the old composition, i.e., outdated events.
MozReview-Commit-ID: BNRcoYxABpd
--HG--
extra : rebase_source : caea469afeed8cc373aeca33199ac0d570052569
ContentCacheInParent::mPendingCommitLength is never initialized until it
receives eCompositionCommit(AsIs) event from widget or receives the latest
content from the remote process when there is a composition.
The bug is, immediately after dispatching eCompositionStart and first
eCompositionChange event, MS Pinyin tries to query the character at caret,
but ContentCacheInParent::HandleQueryContentEvent() tries to resolve
related position of an eQueryTextRect event with the uninitialized
mPendingCommitLength. Therefore, the query almost always fails and MS Pinyin
gives up to show its candidate window.
This patch just initializes the member with 0.
MozReview-Commit-ID: JyYNqi8hoTa
--HG--
extra : rebase_source : bdc504f83abdbb21e11ea69290908ed501e9a65f
For protecting main process, we should stop crashing main process in release
build even when we detect our bug. However, we should keep crashing with
MOZ_DIAGNOSTIC_ASSER which is enabled only on Night and Developer Edition.
MozReview-Commit-ID: 5BQ46IFzXXj
--HG--
extra : rebase_source : 1a894bb23b6b9f386b19eba95d14cd8db80fb2c6
Now, TextComposition::RequestToCommit() manages if it has already requested
IME to commit or cancel composition and this is important for redundant
requests. Therefore, ContentCacheInParent::RequestIMEToCommitComposition()
shouldn't call nsIWidget::NotifyIME() directly.
MozReview-Commit-ID: 69VpgyK9Jk5
--HG--
extra : rebase_source : 5b86c11669c7a69ceb0a2af155765834621ee968
This is a simple bug of ContentCacheInParent. When
ContentCacheInParent::RequestIMEToCommitComposition() returns true,
PuppetWidget::RequestIMEToCommitComposition() will send
eCompositionCommitRequestHandled pseudo event message back to the main process.
This causes counting down mPendingEventsNeedingAck in
ContentCacheInParent::OnEventNeedingAckHandled(). Therefore, in the normal
path, ContentCacheInParent::OnCompositionEvent() increments it for receiving
the pseudo event message.
However, if the tab parent has already lost focus,
RequestIMEToCommitComposition() returns true without requesting native IME to
commit composition. So, ContentCacheInParent::OnCompositionEvent() cannot
increment mPendingEventsNeedingAck for coming
eCompositionCommitRequestHandled. Therefore, RequestIMEToCommitComposition()
needs to increment mPendingEventsNeedingAck by itself when it won't request
IME to commit composition but it returns true.
MozReview-Commit-ID: 4Alwfy8avB
--HG--
extra : rebase_source : 2588221568440beecc2b992910fa53729b8abe1c
This is a follow up patch of bug 1408086. The previous patch starts to append
log of 2 sets of composition events to app notes of crash report when
ContentCacheInParent::OnEventNeedingAckHandled() meets unexpected state and
crash itself. However, now, we know the unexpected state occurs when TabParent
receives eCompositionCommitRequestHandled message from its remote process.
The event comes when ContentCacheInParent::RequestIMEToCommitComposition()
returns true. So, we need to know what occurs in the method before the crash.
This patch defines each case of RequestIMEToCommitComposition() with an enum
class, RequestIMEToCommitCompositionResult and make
RequestIMEToCommitComposition() append one of its value to the array.
Then, ContentCacheInParent discards unnecessary log of this when it discards
log of old composition events. Finally, appends the log to the app notes of
crash report.
MozReview-Commit-ID: 9sJyl4SvUXu
--HG--
extra : rebase_source : f7e90a157d3819523d3d8932d9f8af5d94e2db1f
Masayuki Nakano
Iulian Moraru
Noemi Erli
Christian Holler
Nika Layzell
Narcis Beleuzu
Simon Giesecke
Bogdan Tara
Emilio Cobos Álvarez
Henri Sivonen
Ryan Hunt
Sylvestre Ledru
Milan Sreckovic
Gabriele Svelto