I see that WebViewImpl::removeSpellingMarkersUnderWords is only used from SpellCheck::OnCustomDictionaryChanged which

(1) responds to a message that is send to *all* renderer processes (by SpellcheckService::OnCustomDictionaryChanged)
and
(2) iterates over all local RenderViews

AFAICT, combination of (1) and (2) from above means that all frames should be covered, even though we skip remote frames.  In other words, the code today might be behaving correctly even when OOPIFs are present.

OTOH, I am not sure how to conclusively prove that OOPIFs work.  FWIW, I tried to see what tests would break if I remove the body of WebViewImpl::removeSpellingMarkersUnderWords in https://codereview.chromium.org/2240333002 and all tests passed except for a few unit tests (and I am not convinced that single-process unit tests are sufficient to verify the OOPIF behavior and prevent regressions in the future).  The tests failed in the body-removal-experiment:

All/ParameterizedWebFrameTest.RemoveSpellingMarkersUnderWords/0
All/ParameterizedWebFrameTest.RemoveSpellingMarkersUnderWords/1
All/ParameterizedWebFrameTest.MarkerHashIdentifiers/0
All/ParameterizedWebFrameTest.MarkerHashIdentifiers/1

xiaochengh@: Do you have a sense what the impact of this bug is, now that the other spellcheck logic has been updated?  Just curious if we should prioritize it.  (Thanks for all your work on  issue 638361 !)

Looks like it's used from SpellCheck::OnCustomDictionaryChanged, so I'm guessing it means that if you change your dictionary, we won't clear out the previous red underlines from OOPIFs on the page.  Sound right?

Then again, I can't repro that on 59.0.3071.0.  Typing a word into Subframe2 of http://anforowicz.github.io/undo-test/index.html, then changing languages in chrome://settings to change whether the word is considered wrong, then switching focus back to the tab doesn't show any stale data.  The red underline updates in the OOPIF just as it does on the main page.

Am I missing something?

(Worth noting that I'm using --site-per-process on the test page in comment 4, so Subframe2 is in an OOPIF.)

I have the same guess as in #4. I haven't worked on spellcheck.h/cc, though, so I can't say it for sure.

The steps in #4 might be incorrect, because Blink removes spelling markers from <input>  when it loses focus, which means the markers are already gone when changing the settings.

A correct repro case needs an OOPIF with contenteditable or <textarea>, so that the markers are not removed when losing focus.

Anyway, I agree with the analysis in #1 that the current implementation is already correct, as the browser is broadcasting the message to all renderers.

For code health, I think WebView::removeSpellingMarkersUnderWords should be moved to WebLocalFrame.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45c072e012350840ad94390e71dd25b2329a3c54

commit 45c072e012350840ad94390e71dd25b2329a3c54
Author: xiaochengh <xiaochengh@chromium.org>
Date: Mon Apr 17 21:45:06 2017

Move spelling marker related functions from WebView to WebLocalFrame

Given that spelling markers are managed inside each frame independently,
this patch moves the related functions from WebView to WebLocalFrame for
better architecture.

BUG= 638356 
TEST=n/a; no behavioral change

Review-Url: https://codereview.chromium.org/2816263003
Cr-Commit-Position: refs/heads/master@{#465030}

[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/components/spellcheck/renderer/spellcheck.cc
[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/third_party/WebKit/public/web/WebLocalFrame.h
[modify] https://crrev.com/45c072e012350840ad94390e71dd25b2329a3c54/third_party/WebKit/public/web/WebView.h

As analyzed in #1, this is not actually a bug, but rather a not-so-good design.

With r465030, the function is moved to WebLocalFrame. I don't think there is any remaining issue with the function. Hence marking as fixed.