Depends on D100984

Depends on D100985

https://hg.mozilla.org/mozilla-central/rev/d082bf810441
https://hg.mozilla.org/mozilla-central/rev/1fbf5ecc250e
https://hg.mozilla.org/mozilla-central/rev/f067a137f78b

Depends on D101354

Depends on D101355

https://hg.mozilla.org/mozilla-central/rev/934bd852d994
https://hg.mozilla.org/mozilla-central/rev/dda78af3cdb2
https://hg.mozilla.org/mozilla-central/rev/3ccfbd94c515

Simplifie reasoning about them.

Depends on D101454

Depends on D101571

Backed out 4 changesets (bug 1685303) for bustage complaining about offsets.

Push with failure: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=a4b5ff1a57a130c0116b343b8f3101bb65701343&searchStr=build&tochange=8e6019fb90580edb5be59e72240120610b92a713&selectedTaskRun=NAkWArc-R5-YTNm9C0iQeA.0

Backout link: https://hg.mozilla.org/integration/autoland/rev/8e6019fb90580edb5be59e72240120610b92a713

Failure log: https://treeherder.mozilla.org/logviewer?job_id=326668098&repo=autoland&lineNumber=49127

[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/layout/base'
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_layout_base0.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fstack-clash-protection -ftrivial-auto-var-init=pattern -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/layout/base -I/builds/worker/workspace/obj-build/layout/base -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/layout/forms -I/builds/worker/checkouts/gecko/layout/generic -I/builds/worker/checkouts/gecko/layout/mathml -I/builds/worker/checkouts/gecko/layout/painting -I/builds/worker/checkouts/gecko/layout/printing -I/builds/worker/checkouts/gecko/layout/style -I/builds/worker/checkouts/gecko/layout/tables -I/builds/worker/checkouts/gecko/layout/xul -I/builds/worker/checkouts/gecko/layout/xul/tree -I/builds/worker/checkouts/gecko/docshell/base -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/checkouts/gecko/dom/html -I/builds/worker/checkouts/gecko/dom/svg -I/builds/worker/checkouts/gecko/dom/xul -I/builds/worker/checkouts/gecko/view -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-error=deprecated-copy -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-psabi -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -fexperimental-new-pass-manager  -MD -MP -MF .deps/Unified_cpp_layout_base0.o.pp   Unified_cpp_layout_base0.cpp
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -  In file included from Unified_cpp_layout_base0.cpp:20:
[task 2021-01-14T10:51:08.684Z] 10:51:08    ERROR -  /builds/worker/checkouts/gecko/layout/base/AccessibleCaretManager.cpp:634:13: error: arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument).  'offsets.content' is neither.
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -              offsets.content, offsets.StartOffset(), offsets.EndOffset(),
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -              ^~~~~~~~~~~~~~~
[task 2021-01-14T10:51:08.684Z] 10:51:08    ERROR -  /builds/worker/checkouts/gecko/layout/base/AccessibleCaretManager.cpp:1254:19: error: arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument).  'offsets.content' is neither.
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -    fs->HandleClick(offsets.content, offsets.StartOffset(), offsets.EndOffset(),
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -                    ^~~~~~~~~~~~~~~
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -  2 errors generated.
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -  /builds/worker/checkouts/gecko/config/rules.mk:674: recipe for target 'Unified_cpp_layout_base0.o' failed
[task 2021-01-14T10:51:08.684Z] 10:51:08    ERROR -  make[4]: *** [Unified_cpp_layout_base0.o] Error 1
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/layout/base'
[task 2021-01-14T10:51:08.684Z] 10:51:08     INFO -  make[4]: *** Waiting for unfinished jobs....

:bogdant: thanks for the ni?-request. Analyzing the problem.

Helps distinguishing what modifies frames and what doesn't.

Depends on D101598

Depends on D101751

Depends on D101760

Depends on D101761

https://hg.mozilla.org/mozilla-central/rev/2568089584bf
https://hg.mozilla.org/mozilla-central/rev/9d4477e6aea9
https://hg.mozilla.org/mozilla-central/rev/343c9ac86009
https://hg.mozilla.org/mozilla-central/rev/987a78601024
https://hg.mozilla.org/mozilla-central/rev/27d2b8ca0b4c

https://hg.mozilla.org/mozilla-central/rev/0556bc3c96ba
https://hg.mozilla.org/mozilla-central/rev/b7b01c00850e
https://hg.mozilla.org/mozilla-central/rev/27aba7da0283

Depends on D101931

Now the method states clearer what it does.

Might later help to clean up the dependencies to Selection.

https://hg.mozilla.org/mozilla-central/rev/19f690eae3af
https://hg.mozilla.org/mozilla-central/rev/0d0fd044a0f1

The wording is less ambigous and more const-access is allowed.

Depends on D102301

It reflects clearer what the method does.

Depends on D102302

https://hg.mozilla.org/mozilla-central/rev/f879c2768ec0

Encapsulates flushing-related functionality.

Please see part 25) for further simplifcation of LayoutFlusher.

Depends on D102413

Depends on D102414

https://hg.mozilla.org/mozilla-central/rev/e396d290a9e9
https://hg.mozilla.org/mozilla-central/rev/729ec22edb22
https://hg.mozilla.org/mozilla-central/rev/83ae7e5eba47
https://hg.mozilla.org/mozilla-central/rev/566c7cd855ce
https://hg.mozilla.org/mozilla-central/rev/b9a011762977

Depends on D102606

In the unit-tests, flushing is not done. This was prevented implicitly
by nulling mPresShell. This change makes that explicit by stubbing
MaybeFlushLayout.

Note that IsTerminated, which is called in the same context is already
virtual, so I expect this to not have a noticeable performance-impact.

Depends on D102607

It's clearer and saves one superfluous call to MaybeFlushLayout.

Depends on D102608

https://hg.mozilla.org/mozilla-central/rev/4e10526050f2
https://hg.mozilla.org/mozilla-central/rev/45cf941f54e2
https://hg.mozilla.org/mozilla-central/rev/51ba816c6b8c

https://hg.mozilla.org/mozilla-central/rev/95e93d2cb871

They belong together.

Depends on D102917

Depends on D102918

Will help to simplify AccessibleCaretManager::Dispatch....

Depends on D102919

Preparation to simplify AccessibleCaretManager::Dispatch....

Depends on D102920

Depends on D102921

Will help to hide the UniquePtrs.

Depends on D102922

Depends on D102923

Preparation to hide the UniquePtrs of Carets.

Depends on D102924

Depends on D102925

https://hg.mozilla.org/mozilla-central/rev/d44181196337
https://hg.mozilla.org/mozilla-central/rev/d361437d483b

Now that 86 has moved to Beta, maybe this bug should be closed and the remaining patches moved to another bug. That would help track potential regressions against this bug since it wouldn't span multiple releases.

(In reply to Mathew Hodson from comment #65)

Now that 86 has moved to Beta, maybe this bug should be closed and the remaining patches moved to another bug. That would help track potential regressions against this bug since it wouldn't span multiple releases.

Thanks, it's a valid concern. In this case the 'fortification' is a refactoring, intentionally not changing functionality with mini-reviews. I'll file a new bug for new reviews.

Thanks, it's a valid concern. In this case the 'fortification' is a refactoring, intentionally not changing functionality with mini-reviews. I'll file a new bug for new reviews.

Yes, please move part 27 and later patches to another bug to land them. Ideally, to help sheriff tracking regressions, it's more preferable to land a series of patches in one bug, and clone the bug to land another series of patches if needed.

(In reply to Ting-Yu Lin [:TYLin] (UTC-8) from comment #67)

Thanks, it's a valid concern. In this case the 'fortification' is a refactoring, intentionally not changing functionality with mini-reviews. I'll file a new bug for new reviews.

Yes, please move part 27 and later patches to another bug to land them. Ideally, to help sheriff tracking regressions, it's more preferable to land a series of patches in one bug, and clone the bug to land another series of patches if needed.

Will do so.

Comment on attachment 9199014 [details]
Bug 1685303: part 29) Encapsulate AccessibleCaretManager::mFirstCaret, mSecondCaret in mCarets. r=TYLin

Revision D102917 was moved to bug 1688832. Setting attachment 9199014 [details] to obsolete.

Comment on attachment 9199015 [details]
Bug 1685303: part 30) Add AccessibleCaretManager::Carets::AreLogicallyVisible. r=TYLin

Revision D102918 was moved to bug 1688832. Setting attachment 9199015 [details] to obsolete.

Comment on attachment 9199016 [details]
Bug 1685303: part 31) Add AccessibleCaretManager::Carets::AreVisuallyVisible r=TYLin

Revision D102919 was moved to bug 1688832. Setting attachment 9199016 [details] to obsolete.

Comment on attachment 9199017 [details]
Bug 1685303: part 32) Add AccessibleCaretManager::SelectionStringifyer. r=TYLin

Revision D102920 was moved to bug 1688832. Setting attachment 9199017 [details] to obsolete.

Comment on attachment 9199018 [details]
Bug 1685303: part 33) Add static AccessibleCaretManager::GetSelection, ::GetFrameSelection. r=TYLin

Revision D102921 was moved to bug 1688832. Setting attachment 9199018 [details] to obsolete.

Comment on attachment 9199019 [details]
Bug 1685303: part 34) Prepare AccessibleCaretManager::DispatchCaretStateChangedEvent for splitting. r=TYLin

Revision D102922 was moved to bug 1688832. Setting attachment 9199019 [details] to obsolete.

Comment on attachment 9199020 [details]
Bug 1685303: part 35) Add AccessibleCaretManager::Carets::GetFirst. r=TYLin

Revision D102923 was moved to bug 1688832. Setting attachment 9199020 [details] to obsolete.

Comment on attachment 9199021 [details]
Bug 1685303: part 36) Add AccessibleCaretManager::Carets::GetSecond. r=TYLin

Revision D102924 was moved to bug 1688832. Setting attachment 9199021 [details] to obsolete.

Comment on attachment 9199022 [details]
Bug 1685303: part 37) Add non-default constructor to AccessibleCaretManager::Carets. r=TYLin

Revision D102925 was moved to bug 1688832. Setting attachment 9199022 [details] to obsolete.

Comment on attachment 9199024 [details]
Bug 1685303: part 38) Hide UniquePtrs in AccessibleCaretManager::Carets. r=TYLin

Revision D102927 was moved to bug 1688832. Setting attachment 9199024 [details] to obsolete.