Revert getOffsetForCaret logarithmic search fix

[justinmc]

Reverts https://github.com/flutter/flutter/pull/143954 (originally https://github.com/flutter/flutter/pull/143281, CC @LongCatIsLooong). It's causing some crashes that seem to be coming from this line:

https://github.com/flutter/flutter/blob/e11b5f3cc20abf555e595944acfb295e92182481/packages/flutter/lib/src/painting/text_painter.dart#L1472

I would try to fix the underlying problem, but it's not immediately clear to me what's wrong there.

This was a big revert that conflicted with a few subsequent PRs (I noticed https://github.com/flutter/flutter/commit/3295a48f171980dda669b96b2cd5b49c91fdf469 and https://github.com/flutter/flutter/pull/144308), so those should be looked at when this is relanded.

[LongCatIsLooong]

it's easier to fix this forward. Is there a repro that I can add as a test?

[tugorez]

Is there a repro that I can add as a test?

We don't have any concrete scenarios where this can be reproduced unfortunately. Our monitoring tools flagged this exception.

Is single the right call here? Can you think of scenarios where there might be more than 1 box?

it's easier to fix this forward.

https://github.com/flutter/flutter/wiki/Tree-hygiene?ref=200lab.io#tldr seems to suggest otherwise. It is almost always better to just rollback a regression. If this can't be rolled back without complications I'd suggest making checks before calling single and throwing meaningful information so that we can have a better understanding of what's going on

if (boxes.length != 1) {
  throw 'What is going on? got a looooot of boxes ${generateNiceDebugInfo(boxes)}' ;
}
final TextBox box = boxes.single; 

[LongCatIsLooong]

Yeah the single is too aggressive. We're probably breaking up grapheme clusters. I think I should be able to repro with the text from https://github.com/flutter/flutter/issues/144528 and add a temporary fix for the crash.

The revert has conflicts since it's been two months, and it breaks customer tests, so I think it's riskier (and the fix may need a CP since I think it's in the stable).

[tugorez]

The revert has conflicts since it's been two months, and it breaks customer tests, so I think it's riskier (and the fix may need a CP since I think it's in the stable).

Thank you for providing rationale. Totally makes sense. Agree on fixing forward.

I think I should be able to repro with the text from https://github.com/flutter/flutter/issues/144528 and add a temporary fix for the crash.

Awesome! In parallel I'll work with internal teams to see if we can understand better how / where this is breaking.

Appreciate the help :handshake: !

[tgucio]

We're already handling boxes being empty and a single box - perhaps we should just not assume what the engine will do and handle .length > 1 also which I guess may happen, besides #144528, with odd or buggy fonts. Not sure how to test this though as we call ui.Paragraph.getBoxesForRange().

    if (boxes.isNotEmpty) {
      final TextBox box = boxes.single;
    } else {

[justinmc]

Closing as this has been worked around in https://github.com/flutter/flutter/pull/146669 and should not require a revert.