Test fixes and clean up in preparation for moving DSE to Prefs.

Test fixes and clean up in preparation for moving DSE to Prefs.

TBR= dbeam@chromium.org
(TBR'ing for a pref name change in options UI). 
BUG=365762
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268781

[Cait (Slow), 2014-05-06 15:01:14]

Hello all, 
This change refactors a few tests in preparation for
https://codereview.chromium.org/268643002. Please don't hesitate to ping me if
you have any questions/concerns -- thanks!

pkasting: PTAL at:
   c/b/autocomplete/
   c/b/search_engines/
   c/b/ui/omnibox/

samarth: PTAL at  c/b/ui/browser_instant_controller_unittest.cc

dbeam: PTAL at  c/b/resources/options/ (This change switches the options UI to
listen to a new pref which is now being written alongside the old one by
DefaultSearchPolicyHandler -- we will discontinue writing to the old pref in an
upcoming CL.)

[samarth, 2014-05-06 15:10:55]

lgtm

[Peter Kasting, 2014-05-06 19:49:00]

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/browse...
File chrome/browser/ui/browser_instant_controller_unittest.cc (right):

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/browse...
chrome/browser/ui/browser_instant_controller_unittest.cc:33: class
BrowserInstantControllerEmbeddedTest : public InstantUnitTestBase {
Why not just do this on BrowserInstantControllerTest?  That's what Erik's and my
changes both do, and I think the GoogleBaseURLUpdated test below is going to
need this anyway.

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/search...
File chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
(right):

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/search...
chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc:103:
TemplateURLTableModel* table_model() const {
Nit: While here: const functions should not return non-const pointers

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/search...
chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc:107:
protected:
Nit: While you're cleaning up this file anyway, make these private and give them
accessors as needed.

[Cait (Slow), 2014-05-06 20:10:53]

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/browse...
File chrome/browser/ui/browser_instant_controller_unittest.cc (right):

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/browse...
chrome/browser/ui/browser_instant_controller_unittest.cc:33: class
BrowserInstantControllerEmbeddedTest : public InstantUnitTestBase {
On 2014/05/06 19:49:00, Peter Kasting wrote:
> Why not just do this on BrowserInstantControllerTest?  That's what Erik's and
my
> changes both do, and I think the GoogleBaseURLUpdated test below is going to
> need this anyway.
I tried this and it broke GoogleBaseURLUpdated to have the field trial enabled
-- Perhaps the changes to BrowserInstantController in your CL fix this though?
In that case, I'll let you handle the BrowserInstantController-related test
fixes in your CL.

[Cait (Slow), 2014-05-06 21:47:47]

I've reverted the BrowserInstantController test fix -- it can land in the other
patch

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/search...
File chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
(right):

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/search...
chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc:103:
TemplateURLTableModel* table_model() const {
On 2014/05/06 19:49:00, Peter Kasting wrote:
> Nit: While here: const functions should not return non-const pointers

Done.

https://codereview.chromium.org/265393007/diff/60001/chrome/browser/ui/search...
chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc:107:
protected:
On 2014/05/06 19:49:00, Peter Kasting wrote:
> Nit: While you're cleaning up this file anyway, make these private and give
them
> accessors as needed.

Done.

[Peter Kasting, 2014-05-06 21:49:28]

LGTM

https://codereview.chromium.org/265393007/diff/80001/chrome/browser/ui/search...
File chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc
(right):

https://codereview.chromium.org/265393007/diff/80001/chrome/browser/ui/search...
chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc:107: int
items_changed_count() { return items_changed_count_; }
Nit: These next three can be const.

The ones above can be const iff their returned pointers could also be const.  (I
didn't look at the callers to see.)

[Cait (Slow), 2014-05-06 22:17:02]

The CQ bit was checked by caitkp@chromium.org

[commit-bot: I haz the power, 2014-05-06 22:18:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/caitkp@chromium.org/265393007/100001

[Dan Beam, 2014-05-06 22:48:14]

lgtm

[commit-bot: I haz the power, 2014-05-07 15:30:29]

Change committed as 268781