fix: manually set spellchecker for sub frames

[deepak1556]

Since the spellchecker has been refactored for OOPIF to be frame based instead of view based https://bugs.chromium.org/p/chromium/issues/detail?id=638361 , we need to make sure that its enabled for the sub frames.

Fixes #11147

[ckerr]

The steps look good but there might be a small timing issue here: in subsequent calls, the webframes' TextCheckClient will be dangling pointers inbetween the calls to
spell_check_client_.reset() and
ForEach() ->render_frame->GetWebFrame()->SetTextCheckClient()

Seems like we'd want to keep the previous spell_check_client_ alive in temporary unique_ptr until after ForEach

[ckerr]

Followup question, is it certain that the ForEach will change all the frames that hold a pointer to the old spellchecker?

E.g. is it possible for GetRenderView(), GetMainRenderFrame(), or IsMainFrame() to change so that subsequent calls to ForEach might change a different set of frames than the last call?

[deepak1556]

there might be a small timing issue here:

👍 , in that case would it be better to initialize SpellCheckClient once per frame and just reset the states on subsequent calls ? I am not sure about the reason for current implementation. @zcbenz what do you think about this ?

is it certain that the ForEach will change all the frames that hold a pointer to the old spellchecker?

Yup, ForEach is certain to iterate over all live frames in the process. The only reason for the main frame checks are to make sure that in sandbox mode frames with similar site instances in different windows only get assigned to the spellchecker their parent frame is assigned with.

[zcbenz]

You can keep the new pointer and then do a swap:

std::unique_ptr client(new SpellCheckClient());
ForEach()->Set(client.get());
spell_check_client_.swap(client);

[deepak1556]

Sorry for the delay, have addressed the changes requested. Thanks!

[ckerr]

👍

[zcbenz]

The failing test is not related to this PR, I'm merging anyway.