Add HasWord(string) method to SpellcheckCustomDictionary

Add HasWord(string) method to SpellcheckCustomDictionary

This CL adds a method HasWord(string) to SpellcheckCustomDictionary and changes
the internal data structure in SpellcheckCustomDictionary from a vector to a set
for more efficient querying. Spelling service client will use the new method to
send IN_DICTIONARY feedback to spelling service for words that are in user's
custom spellcheck dictionary. Spelling service client will use this method in
upcoming CLs.

This CL also modifies IPC message "SpellCheckMsg_Init" to pass a set instead of
a vector of custom dictionary words. The browser sends this message to the
renderer to initialize spellcheck.

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

[please use gerrit instead, 2013-05-23 22:54:50]

Rachel: PTAL.

[please use gerrit instead, 2013-05-23 22:56:40]

Justin: PTAL chrome/common/spellcheck_messages.h

[please use gerrit instead, 2013-05-23 22:58:13]

Dan: PTAL chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc

[Dan Beam, 2013-05-24 02:57:51]

c/b/ui/webui/options lgtm

https://chromiumcodereview.appspot.com/15940004/diff/17001/chrome/browser/ui/...
File chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc
(right):

https://chromiumcodereview.appspot.com/15940004/diff/17001/chrome/browser/ui/...
chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc:95: for
(chrome::spellcheck_common::WordSet::iterator it = words.begin();
nit: const_iterator

https://chromiumcodereview.appspot.com/15940004/diff/17001/chrome/browser/ui/...
chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc:95: for
(chrome::spellcheck_common::WordSet::iterator it = words.begin();
wouldn't it be nice to use auto here?!  what a mouthful...

https://chromiumcodereview.appspot.com/15940004/diff/17001/chrome/browser/ui/...
chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc:96: it !=
words.end();
nit: it's probably pretty normal to put

  it != words.end(); ++it) {

on the same line

[please use gerrit instead, 2013-05-24 16:20:00]

https://chromiumcodereview.appspot.com/15940004/diff/17001/chrome/browser/ui/...
File chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc
(right):

https://chromiumcodereview.appspot.com/15940004/diff/17001/chrome/browser/ui/...
chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc:95: for
(chrome::spellcheck_common::WordSet::iterator it = words.begin();
On 2013/05/24 02:57:51, Dan Beam wrote:
> nit: const_iterator

Done.

https://chromiumcodereview.appspot.com/15940004/diff/17001/chrome/browser/ui/...
chrome/browser/ui/webui/options/language_dictionary_overlay_handler.cc:96: it !=
words.end();
On 2013/05/24 02:57:51, Dan Beam wrote:
> nit: it's probably pretty normal to put
> 
>   it != words.end(); ++it) {
> 
> on the same line

Done.

[please use gerrit instead, 2013-05-24 16:20:56]

Fred: PTAL chrome/browser/sync/test/integration/dictionary_helper.cc

[rpetterson, 2013-05-24 20:05:21]

LGTM although I have a question about the test (but it doesn't change the
underlying functionality of the test).

https://codereview.chromium.org/15940004/diff/25001/chrome/browser/spellcheck...
File chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc
(right):

https://codereview.chromium.org/15940004/diff/25001/chrome/browser/spellcheck...
chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc:329:
EXPECT_TRUE(dictionary->AddWord("bar"));
why flip these?

[please use gerrit instead, 2013-05-24 20:31:09]

https://codereview.chromium.org/15940004/diff/25001/chrome/browser/spellcheck...
File chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc
(right):

https://codereview.chromium.org/15940004/diff/25001/chrome/browser/spellcheck...
chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc:329:
EXPECT_TRUE(dictionary->AddWord("bar"));
On 2013/05/24 20:05:21, rpetterson wrote:
> why flip these?

These flips were not necessary, actually, since |dictionary| is now a set. I
flipped the order to correspond with the reversed order in |words| vector below.
That was flipped to correctly compare with GetAllSyncData() vector, which is now
sorted due to the underlying data structure being a set.

[please use gerrit instead, 2013-05-28 18:34:19]

Justin: I've clarified message direction (browser-to-renderer) in
spellcheck_messages.h. Also I've added a paragraph describing the IPC change to
the CL description.

[jschuh, 2013-05-28 18:37:26]

ipc security lgtm. no change in attack surface.

[akalin, 2013-05-28 18:37:59]

+zea, who's a better reviewer

[Nicolas Zea, 2013-05-28 18:48:00]

sync LGTM

[commit-bot: I haz the power, 2013-05-28 18:51:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/15940004/60001

[commit-bot: I haz the power, 2013-05-28 19:22:59]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-05-28 19:29:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/15940004/60001

[commit-bot: I haz the power, 2013-05-28 19:42:16]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-05-28 21:46:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/15940004/60001

[commit-bot: I haz the power, 2013-05-29 00:47:55]

Change committed as 202722