[Spellcheck] Removing the "no more suggestions from Google" line when using the spelling service

[Spellcheck] Removing the "no more suggestions from Google" line when using the spelling service

When using the spelling service, we should not show the "no more suggestions from Google" since that should intelligently figure out the correct suggestion.


BUG=119483
TEST=run using --use-spelling-service and observe that it shows only the single suggestion. run without that flag and observe that it will still show "no more suggestions from Google"


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

[rpetterson, 2012-12-21 03:47:37]

groby -- spellcheck correctness

brettw - tab_contents owners approval

[groby-ooo-7-16, 2012-12-21 22:18:13]

LGTM w/ nits

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/spellchecke...
File chrome/browser/spellchecker/spelling_service_client.cc (right):

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/spellchecke...
chrome/browser/spellchecker/spelling_service_client.cc:103: type_ = type;
Since tag_ and type_ are only needed when we run the callback, would it make
more sense to pre-bind them? (The fact that the callback needs a type_ and a
tag_ is an implementation detail)

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
File chrome/browser/tab_contents/spelling_menu_observer.cc (right):

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
chrome/browser/tab_contents/spelling_menu_observer.cc:89: if
(!useSpellingService) {
Can you add a browsertest?

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
chrome/browser/tab_contents/spelling_menu_observer.cc:292: if (type !=
SpellingServiceClient::SPELLCHECK) {
Can you add a browsertest?

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
File chrome/browser/tab_contents/spelling_menu_observer.h (right):

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
chrome/browser/tab_contents/spelling_menu_observer.h:15: #include
"chrome/browser/spellchecker/spelling_service_client.h"
Prebinding of type would allow us to get rid of this.

[rpetterson, 2013-01-04 00:05:34]

brettw -- ping for owners approval when you have a moment

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/spellchecke...
File chrome/browser/spellchecker/spelling_service_client.cc (right):

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/spellchecke...
chrome/browser/spellchecker/spelling_service_client.cc:103: type_ = type;
On 2012/12/21 22:18:13, groby wrote:
> Since tag_ and type_ are only needed when we run the callback, would it make
> more sense to pre-bind them? (The fact that the callback needs a type_ and a
> tag_ is an implementation detail)

Done.

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
File chrome/browser/tab_contents/spelling_menu_observer.cc (right):

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
chrome/browser/tab_contents/spelling_menu_observer.cc:89: if
(!useSpellingService) {
On 2012/12/21 22:18:13, groby wrote:
> Can you add a browsertest?

Done.

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
chrome/browser/tab_contents/spelling_menu_observer.cc:292: if (type !=
SpellingServiceClient::SPELLCHECK) {
On 2012/12/21 22:18:13, groby wrote:
> Can you add a browsertest?

I think this and the one above both cover the same case, but let me know if you
think an additional browsertest is warranted and if so what it should test. The
added one tests that we do not show "No more spelling suggestions from Google"
after the suggestion.

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
File chrome/browser/tab_contents/spelling_menu_observer.h (right):

https://codereview.chromium.org/11635063/diff/2001/chrome/browser/tab_content...
chrome/browser/tab_contents/spelling_menu_observer.h:15: #include
"chrome/browser/spellchecker/spelling_service_client.h"
On 2012/12/21 22:18:13, groby wrote:
> Prebinding of type would allow us to get rid of this.

Unfortunately not since we still need to include this to get the definition of
the enum SpellingServiceClient::ServiceType since enums can't be forward
declared.

[brettw, 2013-01-04 00:07:59]

lgtm rubberstamp

[commit-bot: I haz the power, 2013-01-04 18:59:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/11635063/8001

[commit-bot: I haz the power, 2013-01-04 20:13:21]

Change committed as 175178