Automation: fix chrome/browser dependency on chrome/test headers.

Automation: fix chrome/browser dependency on chrome/test headers.

This change also fixes a duplication of declarations
introduced by Chrome Frame.

Also, this removes AutocompleteEditProxy and its only two callers;
we now have coverage from browser and unit tests which are much
more solid.

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

[Paweł Hajdan Jr., 2011-06-01 17:40:36]

Please review:

Amit: Chrome Frame
Peter: I'd like to remove AutocompleteMatchData completely (and the only one
test that uses it and is flaky) - does that sound fine to you? We should have
enough coverage from the browser test
Scott: entire CL

[amit, 2011-06-01 17:47:47]

lgtm for Chrome Frame

[Peter Kasting, 2011-06-01 18:39:34]

On 2011/06/01 17:40:36, Paweł Hajdan Jr. wrote:
> Peter: I'd like to remove AutocompleteMatchData completely (and the only one
> test that uses it and is flaky) - does that sound fine to you? We should have
> enough coverage from the browser test

I don't know what uses it and I don't know how well-tested things are with and
without it.  Use your judgement.

http://codereview.chromium.org/7104029/diff/1/chrome/common/automation_messag...
File chrome/common/automation_messages_internal.h (right):

http://codereview.chromium.org/7104029/diff/1/chrome/common/automation_messag...
chrome/common/automation_messages_internal.h:18: 
Nit: Too many blank lines

[Paweł Hajdan Jr., 2011-06-01 19:20:15]

Ah, I just realized I can't really remove AutocompleteMatchData because it's
also used by omnibox_uitest.

http://codereview.chromium.org/7104029/diff/1/chrome/common/automation_messag...
File chrome/common/automation_messages_internal.h (right):

http://codereview.chromium.org/7104029/diff/1/chrome/common/automation_messag...
chrome/common/automation_messages_internal.h:18: 
On 2011/06/01 18:39:34, Peter Kasting wrote:
> Nit: Too many blank lines

This is needed to avoid breaking the reference build, see comment about __LINE__
below (I need to avoid shifting line numbers). If you'd like me to put a comment
like "This line intentionally left blank" I can do that.

[Peter Kasting, 2011-06-01 20:28:32]

LGTM

http://codereview.chromium.org/7104029/diff/1/chrome/common/automation_messag...
File chrome/common/automation_messages_internal.h (right):

http://codereview.chromium.org/7104029/diff/1/chrome/common/automation_messag...
chrome/common/automation_messages_internal.h:18: 
On 2011/06/01 19:20:15, Paweł Hajdan Jr. wrote:
> On 2011/06/01 18:39:34, Peter Kasting wrote:
> > Nit: Too many blank lines
> 
> This is needed to avoid breaking the reference build, see comment about
__LINE__
> below (I need to avoid shifting line numbers).

Then I suggest putting the blank lines below the comment rather than above it.

http://codereview.chromium.org/7104029/diff/13/chrome/common/automation_messa...
File chrome/common/automation_messages.h (right):

http://codereview.chromium.org/7104029/diff/13/chrome/common/automation_messa...
chrome/common/automation_messages.h:211: struct AutocompleteMatchData {
Nit: This used to have a constructor.  It might be nice to still have one.

[Paweł Hajdan Jr., 2011-06-01 20:30:03]

http://codereview.chromium.org/7104029/diff/13/chrome/common/automation_messa...
File chrome/common/automation_messages.h (right):

http://codereview.chromium.org/7104029/diff/13/chrome/common/automation_messa...
chrome/common/automation_messages.h:211: struct AutocompleteMatchData {
On 2011/06/01 20:28:33, Peter Kasting wrote:
> Nit: This used to have a constructor.  It might be nice to still have one.

The constructor can't be in chrome/common because it references code in
chrome/browser. chrome/common can't depend on chrome/browser. And I think the
default constructor was just not used in a meaningful way.

[sky, 2011-06-01 21:00:33]

LGTM

[Peter Kasting, 2011-06-01 21:05:52]

http://codereview.chromium.org/7104029/diff/13/chrome/common/automation_messa...
File chrome/common/automation_messages.h (right):

http://codereview.chromium.org/7104029/diff/13/chrome/common/automation_messa...
chrome/common/automation_messages.h:211: struct AutocompleteMatchData {
On 2011/06/01 20:30:03, Paweł Hajdan Jr. wrote:
> On 2011/06/01 20:28:33, Peter Kasting wrote:
> > Nit: This used to have a constructor.  It might be nice to still have one.
> 
> The constructor can't be in chrome/common because it references code in
> chrome/browser. chrome/common can't depend on chrome/browser.

Then we should move AutocompleteMatch to chrome/common.  It makes no sense to
not put it there but then have a class that copies all its members; that's still
a dependency, it's just one that's easy to break.  We should just eliminate
AutocompleteMatchData and write code that can serialize an AutocompleteMatch
itself and be done with it.

It'd be even nicer to get rid of this from chrome/common entirely and have tests
depend on chrome/browser if need be, but I suspect that's impossible.

[Paweł Hajdan Jr., 2011-06-02 14:00:14]

I decided to just remove AutocompleteEditProxy completely. It's just not worth
to fix, and we already have replacements (browser and unit tests).

Peter, Scott - could you take another look? The changes are trivial, just
removing even more code.

[sky, 2011-06-02 14:51:36]

LGTM, but are you sure pyauto isn't using these?

[Paweł Hajdan Jr., 2011-06-02 14:58:01]

On 2011/06/02 14:51:36, sky wrote:
> LGTM, but are you sure pyauto isn't using these?

pyauto uses JSON hooks: GetOmniboxInfo, SetOmniboxText, OmniboxAcceptInput and
so on. Those are much better done than the code this CL is removing.

[Peter Kasting, 2011-06-02 17:31:12]

LGTM

http://codereview.chromium.org/7104029/diff/16002/chrome/common/automation_me...
File chrome/common/automation_messages_internal.h (right):

http://codereview.chromium.org/7104029/diff/16002/chrome/common/automation_me...
chrome/common/automation_messages_internal.h:767: // This line blank on purpose,
see comment atop file about __LINE__.
Nit: Might as well replace this comment and the one below with "(intentionally
blank line)" so we're consistent with our grep-bait?

[Paweł Hajdan Jr., 2011-06-02 17:43:58]

http://codereview.chromium.org/7104029/diff/16002/chrome/common/automation_me...
File chrome/common/automation_messages_internal.h (right):

http://codereview.chromium.org/7104029/diff/16002/chrome/common/automation_me...
chrome/common/automation_messages_internal.h:767: // This line blank on purpose,
see comment atop file about __LINE__.
On 2011/06/02 17:31:12, Peter Kasting wrote:
> Nit: Might as well replace this comment and the one below with "(intentionally
> blank line)" so we're consistent with our grep-bait?

Done. Please note that those messages are still used; the grep pattern was
mostly intended for removing deprecated messages.