Adding a word to dictionary should remove spelling markers (Chrome part)

Adding a word to dictionary should remove spelling markers 
(Chrome part).
This patch adds code for removing spelling markers under words on 
chrome's side.

BUG=3506
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291232

[Klemen Forstnerič, 2014-08-19 18:51:19]

groby: PTAL Patch Set 2.

[groby-ooo-7-16, 2014-08-19 20:42:05]

This is pretty much OK, I'd just like to hear how it behaves with large numbers
of tabs before I LG

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
File chrome/renderer/spellchecker/spellcheck.cc (right):

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:16: #include
"content/public/renderer/render_frame.h"
Do you need this?

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:20: #include
"third_party/WebKit/public/web/WebFrame.h"
Or this?

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:178: DocumentMarkersRemover
markersRemover(words_added);
I'm slightly queasy about this. OnCustomDictionaryChanged is sent to all
renderers, and this in turn forwards it to all RenderViewHosts. That might be a
bit expensive, since it does string checks on all markers. 

Can you do a test with a large number of tabs that have text input, on different
sites?

(Ideally, I'd like to restrict it to currently visible views and just flag
others for re-check, but that should probably be a separate CL :)

[Klemen Forstnerič, 2014-08-20 20:07:17]

groby: PTAL Patch Set 3. Please also see my comment regarding the speed of this
patch. Thanks! :-)

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
File chrome/renderer/spellchecker/spellcheck.cc (right):

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:16: #include
"content/public/renderer/render_frame.h"
On 2014/08/19 20:42:05, groby wrote:
> Do you need this?

Whoops, this is a relic from when I was still evaluating different solutions to
this bug. Removed.

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:20: #include
"third_party/WebKit/public/web/WebFrame.h"
On 2014/08/19 20:42:05, groby wrote:
> Or this?

Same as above. Removed.

https://codereview.chromium.org/411393004/diff/40001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:178: DocumentMarkersRemover
markersRemover(words_added);
On 2014/08/19 20:42:05, groby wrote:
> I'm slightly queasy about this. OnCustomDictionaryChanged is sent to all
> renderers, and this in turn forwards it to all RenderViewHosts. That might be
a
> bit expensive, since it does string checks on all markers. 
> 
> Can you do a test with a large number of tabs that have text input, on
different
> sites?
> 
> (Ideally, I'd like to restrict it to currently visible views and just flag
> others for re-check, but that should probably be a separate CL :)

I opened 10 tabs, loaded 10 different websites and I pasted a 959 word french
text (which means that almost every word was underlined) to each of these tabs'
input fields. Adding any word on any tab to the dictionary caused the markers to
be removed instantly. :-) Flagging the hidden views sounds like a cool option
though, maybe I could do this for my next patch.

[groby-ooo-7-16, 2014-08-21 03:37:04]

LGTM w/ nit

https://codereview.chromium.org/411393004/diff/60001/chrome/renderer/spellche...
File chrome/renderer/spellchecker/spellcheck.cc (right):

https://codereview.chromium.org/411393004/diff/60001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:74: virtual ~DocumentMarkersRemover()
{}
OVERRIDE, please.

[Klemen Forstnerič, 2014-08-21 16:05:22]

https://codereview.chromium.org/411393004/diff/60001/chrome/renderer/spellche...
File chrome/renderer/spellchecker/spellcheck.cc (right):

https://codereview.chromium.org/411393004/diff/60001/chrome/renderer/spellche...
chrome/renderer/spellchecker/spellcheck.cc:74: virtual ~DocumentMarkersRemover()
{}
On 2014/08/21 03:37:04, groby wrote:
> OVERRIDE, please.

Done.

[groby-ooo-7-16, 2014-08-21 17:18:52]

SLGTM

[please use gerrit instead, 2014-08-21 17:20:26]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2014-08-21 17:23:18]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/klemen.forstneric@gmail.com/411393004/...

[commit-bot: I haz the power, 2014-08-21 23:17:04]

Committed patchset #4 (80001) as 291232