1825693 - Validate assigning content data before calling `ContentCacheInParent::AssignContent`

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, enhancement)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

According to bug 1824143, we could have some scenarios to notify selection out of scope of text content range. Therefore, it's better to validate the ranges before assigning the coming data and notifying IME of the new data. Then, we could avoid the main process crash.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1825693 - Make `ContentCache` stop assigning invalid data r=m_kato!,decoder!

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

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f6d49b8d207
Make `ContentCache` stop assigning invalid data r=m_kato,nika

Comment 3

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4f6d49b8d207

Comment 4

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/32b38d0219db
Backed out changeset 4f6d49b8d207 for ContentCache crashes

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for [@ mozilla::ContentCacheInChild::CacheCaret] crashes:
https://hg.mozilla.org/mozilla-central/rev/32b38d0219db3165eb308874d5a77f4d3610db25

Crash report example: bp-21659bbb-120c-4eac-88f6-965310230515

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #5)

Backed out for [@ mozilla::ContentCacheInChild::CacheCaret] crashes:
https://hg.mozilla.org/mozilla-central/rev/32b38d0219db3165eb308874d5a77f4d3610db25

Crash report example: bp-21659bbb-120c-4eac-88f6-965310230515

Oh, this is one of what I tried to look for incomplete data creation paths.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1825693 - Make `PuppetWidget` stop trying to cache `Selection` directly r=m_kato!

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0646a32d694
Make `ContentCache` stop assigning invalid data r=m_kato,nika
https://hg.mozilla.org/integration/autoland/rev/6a854fc8f509
Make `PuppetWidget` stop trying to cache `Selection` directly r=m_kato

Comment 9

[Iulian Moraru]

Backed out for causing reftest failures related to ContentCache::Selection.

• Push with failures

• Failure log

• also this assertion appeared later

• Backout link

[task 2023-05-19T00:42:33.720Z] 00:42:33     INFO - REFTEST TEST-START | editor/libeditor/crashtests/1402196.html
[task 2023-05-19T00:42:33.731Z] 00:42:33     INFO - REFTEST TEST-LOAD | file:///Z:/task_168445289650720/build/tests/reftest/tests/editor/libeditor/crashtests/1402196.html | 1113 / 3976 (27%)
[task 2023-05-19T00:42:34.071Z] 00:42:34     INFO - Exiting due to channel error.
[task 2023-05-19T00:42:34.076Z] 00:42:34     INFO - Exiting due to channel error.
[task 2023-05-19T00:42:34.080Z] 00:42:34     INFO - Exiting due to channel error.
[task 2023-05-19T00:42:34.212Z] 00:42:34    ERROR - TEST-UNEXPECTED-FAIL | editor/libeditor/crashtests/1402196.html | application terminated with exit code 1
[task 2023-05-19T00:42:34.222Z] 00:42:34     INFO - REFTEST INFO | Downloading symbols from: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/EO0xBd14TZm2yoVUXXcoZQ/artifacts/public/build/target.crashreporter-symbols.zip
[task 2023-05-19T00:45:04.692Z] 00:45:04     INFO - REFTEST INFO | Copy/paste: Z:/task_168445289650720/fetches\minidump-stackwalk\minidump-stackwalk.exe --symbols-url=https://symbols.mozilla.org/ --cyborg=C:\Users\task_168445289650720\AppData\Local\Temp\tmp0pw40c_4\07bd23d6-f41d-4831-ba86-afebf8164da0.trace C:\Users\task_168445289650720\AppData\Local\Temp\tmp7r6zh_gu.mozrunner\minidumps\07bd23d6-f41d-4831-ba86-afebf8164da0.dmp C:\Users\task_168445289650720\AppData\Local\Temp\tmpqhm_4bv2
[task 2023-05-19T00:45:18.979Z] 00:45:18     INFO - REFTEST INFO | Saved minidump as Z:\task_168445289650720\build\blobber_upload_dir\07bd23d6-f41d-4831-ba86-afebf8164da0.dmp
[task 2023-05-19T00:45:18.990Z] 00:45:18     INFO - REFTEST INFO | Saved app info as Z:\task_168445289650720\build\blobber_upload_dir\07bd23d6-f41d-4831-ba86-afebf8164da0.extra
[task 2023-05-19T00:45:19.432Z] 00:45:19     INFO - REFTEST PROCESS-CRASH | editor/libeditor/crashtests/1402196.html | application crashed [@ mozilla::Maybe<mozilla::ContentCache::Selection>::operator->]
[task 2023-05-19T00:45:19.432Z] 00:45:19     INFO - Mozilla crash reason: MOZ_RELEASE_ASSERT(isSome())
[task 2023-05-19T00:45:19.432Z] 00:45:19     INFO - Crash dump filename: C:\Users\task_168445289650720\AppData\Local\Temp\tmp7r6zh_gu.mozrunner\minidumps\07bd23d6-f41d-4831-ba86-afebf8164da0.dmp
[task 2023-05-19T00:45:19.432Z] 00:45:19     INFO - Operating system: Windows NT
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -                   10.0.22621
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO - CPU: x86
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      GenuineIntel family 6 model 106 stepping 6
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      8 CPUs
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO - 
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO - Crash reason:  EXCEPTION_BREAKPOINT
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO - Crash address: 0x6bed0730
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO - Process uptime: 174 seconds
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO - 
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO - Thread 0 MainThread (crashed)
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -  0  xul.dll!mozilla::Maybe<mozilla::ContentCache::Selection>::operator->() const [Maybe.h:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 789]
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -     Found by: inlining
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -  1  xul.dll!mozilla::ContentCacheInParent::HandleQueryContentEvent(mozilla::WidgetQueryContentEvent&, nsIWidget*) const [ContentCache.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 860 + 0x15]
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      eip = 0x6bed0730    esp = 0x051dd1a4    ebp = 0x051dd210    ebx = 0x051dd1e8
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      esi = 0x6fac3f90    edi = 0x051dd5c8    eax = 0x7127e9ac    ecx = 0x00000001
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      edx = 0x6fac3f90 eflags = 0x00000246
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -     Found by: given as instruction pointer in context
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -  2  xul.dll!mozilla::dom::BrowserParent::HandleQueryContentEvent(mozilla::WidgetQueryContentEvent&) [BrowserParent.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 3080 + 0xc]
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      eip = 0x6bb4bccc    esp = 0x051dd218    ebp = 0x051dd244    ebx = 0x051dd5c8
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      esi = 0x2277fa00    edi = 0x19fe1000
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -  3  xul.dll!mozilla::EventStateManager::PreHandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIFrame*, nsIContent*, nsEventStatus*, nsIContent*) [EventStateManager.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 644 + 0x18]
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      eip = 0x6b2cc608    esp = 0x051dd24c    ebp = 0x051dd3bc    ebx = 0x051dd5c8
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -      esi = 0x69bf2e00    edi = 0x00000000
[task 2023-05-19T00:45:19.448Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -  4  xul.dll!mozilla::PresShell::EventHandler::DispatchEvent(mozilla::EventStateManager*, mozilla::WidgetEvent*, bool, nsEventStatus*, nsIContent*) [PresShell.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 8237 + 0x16]
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      eip = 0x6c18e5f7    esp = 0x051dd3c4    ebp = 0x051dd3fc    ebx = 0x155529a0
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      esi = 0x1c2baf00    edi = 0x00000000
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -  5  xul.dll!mozilla::PresShell::EventHandler::HandleEventWithCurrentEventInfo(mozilla::WidgetEvent*, nsEventStatus*, bool, nsIContent*) [PresShell.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 8206 + 0x16]
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      eip = 0x6c18bebb    esp = 0x051dd404    ebp = 0x051dd46c    ebx = 0x051dd4ec
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      esi = 0x1c0eaa60    edi = 0x051dd5c8
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -  6  xul.dll!mozilla::PresShell::EventHandler::HandleEventAtFocusedContent(mozilla::WidgetGUIEvent*, nsEventStatus*) [PresShell.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 7952 + 0x10]
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      eip = 0x6c18c204    esp = 0x051dd474    ebp = 0x051dd4a4    ebx = 0x1284fc00
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      esi = 0x155529a0    edi = 0x051dd4ec
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -  7  xul.dll!mozilla::PresShell::EventHandler::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*) [PresShell.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 6978 + 0x8]
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      eip = 0x6c18b06d    esp = 0x051dd4ac    ebp = 0x051dd4d0    ebx = 0x00000000
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      esi = 0x051dd5c8    edi = 0x051dd4ec
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -  8  xul.dll!mozilla::PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*) [PresShell.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 6896 + 0xe]
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      eip = 0x6c18a9eb    esp = 0x051dd4d8    ebp = 0x051dd504    ebx = 0x1c56a010
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      esi = 0x051dd5c8    edi = 0x00000000
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -  9  xul.dll!nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) [nsViewManager.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 678 + 0xd]
[task 2023-05-19T00:45:19.449Z] 00:45:19     INFO -      eip = 0x6bebc6f2    esp = 0x051dd50c    ebp = 0x051dd530    ebx = 0x1c56a010
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      esi = 0x051dd544    edi = 0x1284fc00
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO - 10  xul.dll!nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) [nsView.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 1149 + 0xa]
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      eip = 0x6bebc5c7    esp = 0x051dd538    ebp = 0x051dd558    ebx = 0x051dd5c8
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      esi = 0x19ff76d0    edi = 0x69bf2e00
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO - 11  xul.dll!nsWindow::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) [nsWindow.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 4399 + 0x27]
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      eip = 0x6bfaa1b1    esp = 0x051dd560    ebp = 0x051dd578    ebx = 0x6bebc530
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      esi = 0x2277fa00    edi = 0x1a69c9a0
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO - 12  xul.dll!nsBaseWidget::DispatchWindowEvent(mozilla::WidgetGUIEvent&) [nsBaseWidget.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 1319 + 0x18]
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      eip = 0x6bec69f1    esp = 0x051dd580    ebp = 0x051dd59c    ebx = 0x051dd5c8
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      esi = 0x2277fa00    edi = 0x6bfaa170
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO - 13  xul.dll!mozilla::widget::TSFTextStore::DispatchEvent(mozilla::WidgetGUIEvent&) [TSFTextStore.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 2209]
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -     Found by: inlining
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO - 14  xul.dll!mozilla::widget::TSFTextStore::GetTextExt(unsigned long, long, long, tagRECT*, int*) [TSFTextStore.cpp:6a854fc8f509f7e9dfc3bd526abc04199b095517 : 4513 + 0x37]
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      eip = 0x6bf2eea3    esp = 0x051dd5a4    ebp = 0x051dd730    ebx = 0x051dd5c8
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      esi = 0x05778700    edi = 0x69bf2e00
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO - 15  textinputframework.dll!CACPWrap::GetTextExt(unsigned long, IAnchor*, IAnchor*, tagRECT*, int*) + 0x7d
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      eip = 0x690328de    esp = 0x051dd738    ebp = 0x051dd760    ebx = 0x051dd824
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      esi = 0x6bf2e940    edi = 0x051dd7dc
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO - 16  textinputframework.dll!CContextView::GetTextExt(unsigned long, ITfRange*, tagRECT*, int*) + 0x9f
[task 2023-05-19T00:45:19.450Z] 00:45:19     INFO -      eip = 0x6903d1e0    esp = 0x051dd768    ebp = 0x051dd790    ebx = 0x051dd824
[task 2023-05-19T00:45:19.451Z] 00:45:19     INFO -      esi = 0x6bf2e940    edi = 0x051dd7dc
[task 2023-05-19T00:45:19.451Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.451Z] 00:45:19     INFO - 17  textinputframework.dll!CInputContextAdapter::GetLayoutBounds(unsigned int, EditControlRange const&, TextRect*, TextRect*) + 0xda
[task 2023-05-19T00:45:19.451Z] 00:45:19     INFO -      eip = 0x6901c4cb    esp = 0x051dd798    ebp = 0x051dd848    ebx = 0x051dd824
[task 2023-05-19T00:45:19.451Z] 00:45:19     INFO -      esi = 0x6bf2e940    edi = 0x051dd7dc
[task 2023-05-19T00:45:19.451Z] 00:45:19     INFO -     Found by: call frame info
[task 2023-05-19T00:45:19.451Z] 00:45:19     INFO - 

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh?? I don't understand the crash... The crash line is here, however, mSelection is checked above...

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, the line number is broken, it's actually crashed here. That makes sense.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1825693 - Make `ContentCacheInChild::CacheTextRects` stop emplacing `Selection` r=m_kato!

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.

Depends on D178145

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a583f2a81d36
Make `ContentCache` stop assigning invalid data r=m_kato,nika
https://hg.mozilla.org/integration/autoland/rev/6a53913ff69c
Make `PuppetWidget` stop trying to cache `Selection` directly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/9d1123793cf4
Make `ContentCacheInChild::CacheTextRects` stop emplacing `Selection` r=m_kato

Comment 14

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/a583f2a81d36
https://hg.mozilla.org/mozilla-central/rev/6a53913ff69c
https://hg.mozilla.org/mozilla-central/rev/9d1123793cf4

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

It seems that there is still a path to make invalid data relation.
https://crash-stats.mozilla.org/report/index/0f49439b-0c8a-4443-8f6e-8e0a20230701