Fix linking errors for browser_tests when autofill_dialog is disabled

Fix linking errors for browser_tests when autofill_dialog is disabled

Fix linking errors for browser_tests when autofill_dialog is disabled due
to the utofill_dialog_controller_browsertest test being built even when
enable_autofill_dialog==0.

BUG=

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

[do-not-use, 2013-08-27 09:40:24]

I was getting the following linking errors on Linux otherwise:
http://pastebin.com/eqMKpzSr

build cmd:
ninja -C out/Debug browser_tests

[Nico, 2013-08-27 14:57:34]

This lgtm, but I'd expect enable_autofill_dialog to go away soonish (right?) so
maybe just don't disable it?

(Generally, the more settings you change from your default, the less likely your
build will work. We should strive to minimize build settings for this reason.)

[do-not-use, 2013-08-27 16:11:26]

On 2013/08/27 14:57:34, Nico wrote:
> This lgtm, but I'd expect enable_autofill_dialog to go away soonish (right?)
so
> maybe just don't disable it?
> 
> (Generally, the more settings you change from your default, the less likely
your
> build will work. We should strive to minimize build settings for this reason.)

I *am* using the default build settings, I am not sure why
enable_autofill_dialog is set to 0.
I am using ninja on Ubuntu Linux 13.04.

[Nico, 2013-08-27 16:27:55]

On 2013/08/27 16:11:26, Christophe Dumez wrote:
> On 2013/08/27 14:57:34, Nico wrote:
> > This lgtm, but I'd expect enable_autofill_dialog to go away soonish (right?)
> so
> > maybe just don't disable it?
> > 
> > (Generally, the more settings you change from your default, the less likely
> your
> > build will work. We should strive to minimize build settings for this
reason.)
> 
> I *am* using the default build settings, I am not sure why
> enable_autofill_dialog is set to 0.
> I am using ninja on Ubuntu Linux 13.04.

The linux debug bot is green:
http://build.chromium.org/p/chromium.linux/builders/Linux%20Builder%20%28dbg%...
It doesn't set anything non-default for enable_autofill_dialog:
http://build.chromium.org/p/chromium.mac/builders/Mac%20Builder%20%28dbg%29/b...

It does compile autofill_dialog_controller_browsertest.cc, and seems to link
fine. It doesn't compile the _impl.cc though, so I don't understand why (see
e.g. clobber build at
http://build.chromium.org/p/chromium/builders/Linux/builds/42281/steps/compil...

…ah, it looks like all tests are in a `#if defined(TOOLKIT_VIEWS) ||
defined(OS_MACOSX)` in the _browsertest.cc file, but the supporting code isn't,
and the file relies on the linker stripping the unused classes. So this CL looks
like a better fix, lgtm.

Maybe cc groby so she's aware of this change (she has a FIXME in the test file).

Thanks for fixing this!

[do-not-use, 2013-08-27 16:31:01]

On 2013/08/27 16:11:26, Christophe Dumez wrote:
> On 2013/08/27 14:57:34, Nico wrote:
> > This lgtm, but I'd expect enable_autofill_dialog to go away soonish (right?)
> so
> > maybe just don't disable it?
> > 
> > (Generally, the more settings you change from your default, the less likely
> your
> > build will work. We should strive to minimize build settings for this
reason.)
> 
> I *am* using the default build settings, I am not sure why
> enable_autofill_dialog is set to 0.
> I am using ninja on Ubuntu Linux 13.04.

according to build/common.gypi, autofill_dialog is disabled by default and
enabled only for:
[toolkit_views==1 or (OS=="android" and android_webview_build==0) or OS=="mac"]

toolkit_views is enabled for ['OS=="win" or chromeos==1 or use_aura==1'] so it
looks like Linux does not enable autofill_dialog?

[do-not-use, 2013-08-27 16:34:05]

+groby

[commit-bot: I haz the power, 2013-08-27 16:51:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23563002/1

[groby-ooo-7-16, 2013-08-27 18:19:36]

LGTM

Nico: enable_autofill_dialog will probably not go away until we have a Gtk
implementation, at least. And even after that, Android will obviously not
include the autofill dialog code. I'm not sure if plan to just gate on OS, or if
the flag is supposed to stay around.

Evan, any comment?

[Evan Stade, 2013-08-27 21:48:34]

I think keeping the feature flag is the nice new standard way of doing things.
If you gate on OS you might have to update some #ifdefs in the code in addition
to some gypi files every time an OS gains or loses a feature.

[commit-bot: I haz the power, 2013-08-28 01:19:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23563002/1

[commit-bot: I haz the power, 2013-08-28 03:01:42]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-28 07:32:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23563002/1

[commit-bot: I haz the power, 2013-08-28 11:08:27]

Change committed as 219992