Even more test cleanup. Some fixes to non-test code.

Even more test cleanup. Some fixes to non-test code that's regressed.

BUG=none
TEST=compiles

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

[Elliot Glaysher, 2011-02-15 21:16:41]

http://codereview.chromium.org/6523032/diff/1/chrome/browser/autofill/billing...
File chrome/browser/autofill/billing_address.h (left):

http://codereview.chromium.org/6523032/diff/1/chrome/browser/autofill/billing...
chrome/browser/autofill/billing_address.h:6: #define
CHROME_BROWSER_AUTOFILL_BILLING_ADDRESS_H_
Checked with David. This just was never GCed.

[Nico, 2011-02-15 21:25:37]

LG

couple nits

http://codereview.chromium.org/6523032/diff/1/chrome/browser/content_settings...
File chrome/browser/content_settings/content_settings_base_provider.cc (right):

http://codereview.chromium.org/6523032/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/content_settings_base_provider.cc:37:
content_settings_for_resources(rhs.content_settings_for_resources) {
this (can't rely on implicit copy constructors) is a bit of a bummer

http://codereview.chromium.org/6523032/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/content_settings_base_provider.cc:43: :
is_off_the_record_(is_otr) {
indent only 4

http://codereview.chromium.org/6523032/diff/1/chrome/browser/geolocation/arbi...
File chrome/browser/geolocation/arbitrator_dependency_factories_for_test.cc
(right):

http://codereview.chromium.org/6523032/diff/1/chrome/browser/geolocation/arbi...
chrome/browser/geolocation/arbitrator_dependency_factories_for_test.cc:8:
GeolocationArbitratorDependencyFactoryWithLocationProvider(
indent 4? also below.

this is a terrible class name

http://codereview.chromium.org/6523032/diff/1/chrome/test/sync/test_http_brid...
File chrome/test/sync/test_http_bridge_factory.h (right):

http://codereview.chromium.org/6523032/diff/1/chrome/test/sync/test_http_brid...
chrome/test/sync/test_http_bridge_factory.h:45: virtual const std::string
GetResponseHeaderValue(const std::string &) const;
i guess this should be std::string&?

http://codereview.chromium.org/6523032/diff/1/jingle/notifier/listener/mediat...
File jingle/notifier/listener/mediator_thread_mock.h (right):

http://codereview.chromium.org/6523032/diff/1/jingle/notifier/listener/mediat...
jingle/notifier/listener/mediator_thread_mock.h:52: // Intneral State
*Internal

[Lei Zhang, 2011-02-15 21:28:41]

LGTM with one minor comment below.

[Lei Zhang, 2011-02-15 21:33:44]

http://codereview.chromium.org/6523032/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/6523032/diff/1/chrome/chrome_tests.gypi#newcode95
chrome/chrome_tests.gypi:95: 'browser/sync/engine/read_node_mock.h',
Shouldn't these 2 be in sync_unit_tests? I don't think anyone else uses them.

[Elliot Glaysher, 2011-02-15 22:09:51]

http://codereview.chromium.org/6523032/diff/1/chrome/browser/content_settings...
File chrome/browser/content_settings/content_settings_base_provider.cc (right):

http://codereview.chromium.org/6523032/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/content_settings_base_provider.cc:37:
content_settings_for_resources(rhs.content_settings_for_resources) {
On 2011/02/15 21:25:37, Nico wrote:
> this (can't rely on implicit copy constructors) is a bit of a bummer

map of string to map of pair of enum and string to arbitrary array?

We need to go deeper.

http://codereview.chromium.org/6523032/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/6523032/diff/1/chrome/chrome_tests.gypi#newcode95
chrome/chrome_tests.gypi:95: 'browser/sync/engine/read_node_mock.h',
On 2011/02/15 21:33:44, Lei Zhang wrote:
> Shouldn't these 2 be in sync_unit_tests? I don't think anyone else uses them.

Was confused. sync_unit_tests *doesn't* use them while unit_tests does. Fixed by
moving to unit_tests instead.

http://codereview.chromium.org/6523032/diff/1/chrome/test/sync/test_http_brid...
File chrome/test/sync/test_http_bridge_factory.h (right):

http://codereview.chromium.org/6523032/diff/1/chrome/test/sync/test_http_brid...
chrome/test/sync/test_http_bridge_factory.h:45: virtual const std::string
GetResponseHeaderValue(const std::string &) const;
On 2011/02/15 21:25:37, Nico wrote:
> i guess this should be std::string&?

Can't; this function is defined by the protobufs and I'm not touching those.
Also, the implementation doesn't return a reference to a real string, instead
copying.