Indicate -> Indicates

I figure this assert in assert is more informative than return false

you could also replace the entire if with return !_debugChildIntegrityEnabled || _debugDanglingKeepAlives.isEmpty

certainly in any case you don't want both ! and isNotEmpty.

if you do the bool thing below, this would become:

return _debugVerifyChildOrder() && (!_debugChildIntegrityEnabled || _debugDanglingKeepAlives.isEmpty);

deal with under flow case

hat do you mean by "there should be no swapping"?

nit: extraneous blank line

(general advice: it's often useful to self-review a patch to catch this kind of thing.)

you can rephrase this as the following for less code duplication:

RenderBox child = firstChild;
int index;
while (child != null) {
  index = indexOf(child);
  child = childAfter(child);
  assert(child == null || indexOf(child) > index);
}

(also removes the need for the if)

you only call this from asserts, so you could make it return bool instead (return null in release builds), and have more idiomatic call sites.

for example then this just becomes assert(_debugVerifyChildOrder());

why?

It is possible that if we have only one child in the render tree with slot 0 and is in our viewport.
If we move the child to slot1.
The super.move will not do anything nor markNeedsLayout since there is only one child and the position does not change in the childList, but we still need to relayout the screen since that child is no longer in slot 0.

grammar nits:
in the childList
call super.move (no the)
with the new slot

it's clearer to write this as:

  childManager.didAdoptChild(child); // updates the slot in the parentData

please update the documentation for didAdoptChild to mention this new contract.

I will update the first sentence, but I think the original documentation is good enough?

/// Subclasses must ensure that the [SliverMultiBoxAdaptorParentData.index]
/// field of the child's [RenderObject.parentData] accurately reflects the
/// child's index in the child list after this function returns.

this code doesn't make sense. (it already didn't make sense before your change. you're not making it worse.)

It means that while a warp is under way, we stop updating the tab page contents.

we stop it here and rely on line 1233 to set it back I presume? It make sense to me although it is not a clean solution

The problem is that if, say, there was an animation being driven in the tab, it would suddenly stop happening while you were switching tabs.

We really should fix this.

Let's keep that a separate pr. This pr is already too complicated

Can you file an issue for changing that? I agree that it should be a separate PR since this one is already complicated.

#31269

Can you file an issue for changing that? I agree that it should be a separate PR since this one is already complicated.

.. and reinsert back to _keepAliveBucket in the new slot.

fixed