[zoom bubble] Add tests for ZoomController::UpdateState() handling empty hosts.

[zoom bubble] Add tests for ZoomController::UpdateState() handling empty hosts.

R=estade@chromium.org
BUG=161388
TEST=unit_tests --gtest_filter='ZoomControllerTest.*' passes on bots.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168677

[Dan Beam, 2012-11-16 04:17:48]

https://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoo...
File chrome/browser/ui/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoo...
chrome/browser/ui/zoom/zoom_controller.cc:82: if (!host.empty()) {
I included this diff (from: https://codereview.chromium.org/11299014/) so I can
try the logic while that CL isn't landed, you can just ignore and I'll rebase
before landing.

[Evan Stade, 2012-11-16 18:34:43]

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
File chrome/browser/ui/zoom/zoom_controller_unittest.cc (right):

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:17: typedef
ChromeRenderViewHostTestHarness ZoomControllerTest;
instead of just a typedef, create a ZoomControllerTest class that does the
initial setup for each test.

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:44:
zoom_controller.OnPreferenceChanged(NULL, prefs::kDefaultZoomLevel);
could you actually change the preference instead of calling OnPreferenceChanged
directly?

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:56:
zoom_controller.Observe(content::NOTIFICATION_ZOOM_LEVEL_CHANGED,
likewise, is it possible to get the HostZoomMap and call SetZoomLevel?

[Dan Beam, 2012-11-17 01:20:32]

+thestig@ for chrome/chrome_tests_unit.gypi

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
File chrome/browser/ui/zoom/zoom_controller_unittest.cc (right):

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:17: typedef
ChromeRenderViewHostTestHarness ZoomControllerTest;
On 2012/11/16 18:34:43, Evan Stade wrote:
> instead of just a typedef, create a ZoomControllerTest class that does the
> initial setup for each test.

Done.

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:44:
zoom_controller.OnPreferenceChanged(NULL, prefs::kDefaultZoomLevel);
On 2012/11/16 18:34:43, Evan Stade wrote:
> could you actually change the preference instead of calling
OnPreferenceChanged
> directly?

Done.

http://codereview.chromium.org/11414018/diff/2002/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:56:
zoom_controller.Observe(content::NOTIFICATION_ZOOM_LEVEL_CHANGED,
On 2012/11/16 18:34:43, Evan Stade wrote:
> likewise, is it possible to get the HostZoomMap and call SetZoomLevel?

Done.

[Evan Stade, 2012-11-17 01:33:06]

http://codereview.chromium.org/11414018/diff/1016/chrome/browser/ui/zoom/zoom...
File chrome/browser/ui/zoom/zoom_controller_unittest.cc (right):

http://codereview.chromium.org/11414018/diff/1016/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:36: void SetUp() OVERRIDE {
shouldn't this be virtual void SetUp() OVERRIDE?

http://codereview.chromium.org/11414018/diff/1016/chrome/browser/ui/zoom/zoom...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:42: void TearDown() OVERRIDE
{
ditto

[Evan Stade, 2012-11-17 01:34:02]

aside from that, LGTM + thanks for writing test.

[Lei Zhang, 2012-11-17 01:39:15]

lgtm on the .gypi file, though you don't need that anymore per the recent PSA.

[Dan Beam, 2012-11-17 01:53:42]

https://chromiumcodereview.appspot.com/11414018/diff/1016/chrome/browser/ui/z...
File chrome/browser/ui/zoom/zoom_controller_unittest.cc (right):

https://chromiumcodereview.appspot.com/11414018/diff/1016/chrome/browser/ui/z...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:36: void SetUp() OVERRIDE {
On 2012/11/17 01:33:06, Evan Stade wrote:
> shouldn't this be virtual void SetUp() OVERRIDE?

Done.

https://chromiumcodereview.appspot.com/11414018/diff/1016/chrome/browser/ui/z...
chrome/browser/ui/zoom/zoom_controller_unittest.cc:42: void TearDown() OVERRIDE
{
On 2012/11/17 01:33:06, Evan Stade wrote:
> ditto

Done.

[commit-bot: I haz the power, 2012-11-17 01:59:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/11414018/9004

[commit-bot: I haz the power, 2012-11-17 02:52:38]

Retried try job too often for step(s) content_unittests

[commit-bot: I haz the power, 2012-11-19 22:55:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/11414018/6006

[commit-bot: I haz the power, 2012-11-20 01:06:04]

Change committed as 168677