AU: Full proxy support

AU: Full proxy support

This CL Introduces a new proxy resolver that queries Chrome browser
for all URLs. This means the updater now works with Proxy
AutoConfiguration (PAC) files and automatic proxy detection.

BUG=chromium-os:9478
TEST=performed update, unittests

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=000d895

[petkov, 2011-02-28 08:14:18]

http://codereview.chromium.org/6594025/diff/1/UpdateEngine.conf
File UpdateEngine.conf (right):

http://codereview.chromium.org/6594025/diff/1/UpdateEngine.conf#newcode31
UpdateEngine.conf:31: <allow 
send_interface="org.chromium.UpdateEngineLibcrosProxyResolvedInterface" />
extra space after allow?

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.cc
File chrome_browser_proxy_resolver.cc (right):

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:6: #include <map>
add empty line before

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:38:
"interface='org.chromium.UpdateEngineLibcrosProxyResolvedInterface'";
why don't you include the signal name in the filter?

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:44: DBusGConnection* bus;
move down to initialization

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:45: DBusGProxy* proxy;
move down to initialization

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:94: initialized_ = true;
style says that this sort of initialization should be done in a separate Init
method that can return success/failure. if it's too much work to refactor now,
maybe at least add a TODO.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.h
File chrome_browser_proxy_resolver.h (right):

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.h...
chrome_browser_proxy_resolver.h:29: FRIEND_TEST(ChromeBrowserProxyResolverTest,
ParseTest);
move these declarations to right after private

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.h...
chrome_browser_proxy_resolver.h:51: private:
add an empty line before

[adlr, 2011-03-01 20:39:13]

Made the corrections. PTAL. Thanks

http://codereview.chromium.org/6594025/diff/1/UpdateEngine.conf
File UpdateEngine.conf (right):

http://codereview.chromium.org/6594025/diff/1/UpdateEngine.conf#newcode31
UpdateEngine.conf:31: <allow 
send_interface="org.chromium.UpdateEngineLibcrosProxyResolvedInterface" />
On 2011/02/28 08:14:18, petkov wrote:
> extra space after allow?

Done.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.cc
File chrome_browser_proxy_resolver.cc (right):

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:6: #include <map>
On 2011/02/28 08:14:18, petkov wrote:
> add empty line before

Done.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:38:
"interface='org.chromium.UpdateEngineLibcrosProxyResolvedInterface'";
On 2011/02/28 08:14:18, petkov wrote:
> why don't you include the signal name in the filter?

Done.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:44: DBusGConnection* bus;
On 2011/02/28 08:14:18, petkov wrote:
> move down to initialization

Done.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:45: DBusGProxy* proxy;
On 2011/02/28 08:14:18, petkov wrote:
> move down to initialization

Done.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.c...
chrome_browser_proxy_resolver.cc:94: initialized_ = true;
On 2011/02/28 08:14:18, petkov wrote:
> style says that this sort of initialization should be done in a separate Init
> method that can return success/failure. if it's too much work to refactor now,
> maybe at least add a TODO.

Done.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.h
File chrome_browser_proxy_resolver.h (right):

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.h...
chrome_browser_proxy_resolver.h:29: FRIEND_TEST(ChromeBrowserProxyResolverTest,
ParseTest);
On 2011/02/28 08:14:18, petkov wrote:
> move these declarations to right after private

Done.

http://codereview.chromium.org/6594025/diff/1/chrome_browser_proxy_resolver.h...
chrome_browser_proxy_resolver.h:51: private:
On 2011/02/28 08:14:18, petkov wrote:
> add an empty line before

Done.

[petkov, 2011-03-01 21:59:20]

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
File chrome_browser_proxy_resolver.cc (right):

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.cc:186: ;
remove?

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.cc:188: CallbacksMap::iterator it =
callbacks_.lower_bound(source_url);
why lower_bound rather than find?

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.cc:196: TimeoutsMap::iterator it =
timers_.lower_bound(source_url);
why lower_bound rather than find?

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolver.h
File chrome_browser_proxy_resolver.h (right):

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.h:29: typedef std::multimap<std::string,
std::pair<ProxiesResolvedFn, void*> >
move typedefs to after private (or after public if they need to be public)

http://codereview.chromium.org/6594025/diff/4001/update_attempter.cc
File update_attempter.cc (right):

http://codereview.chromium.org/6594025/diff/4001/update_attempter.cc#newcode129
update_attempter.cc:129: if (!chrome_proxy_resolver_initialized_) {
does it make more sense to keep this flag in the ChromeProxyResolver class and
check it in Init?

[adlr, 2011-03-02 01:41:25]

PTAL; thanks!

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
File chrome_browser_proxy_resolver.cc (right):

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.cc:186: ;
On 2011/03/01 21:59:20, petkov wrote:
> remove?

Done.

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.cc:188: CallbacksMap::iterator it =
callbacks_.lower_bound(source_url);
On 2011/03/01 21:59:20, petkov wrote:
> why lower_bound rather than find?

I want the first element whose key matches, but i think the contract for find()
is that it will find any such element.

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.cc:196: TimeoutsMap::iterator it =
timers_.lower_bound(source_url);
On 2011/03/01 21:59:20, petkov wrote:
> why lower_bound rather than find?

I want the first element whose key matches, but i think the contract for find()
is that it will find any such element.

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolver.h
File chrome_browser_proxy_resolver.h (right):

http://codereview.chromium.org/6594025/diff/4001/chrome_browser_proxy_resolve...
chrome_browser_proxy_resolver.h:29: typedef std::multimap<std::string,
std::pair<ProxiesResolvedFn, void*> >
On 2011/03/01 21:59:20, petkov wrote:
> move typedefs to after private (or after public if they need to be public)

Done.

http://codereview.chromium.org/6594025/diff/4001/update_attempter.cc
File update_attempter.cc (right):

http://codereview.chromium.org/6594025/diff/4001/update_attempter.cc#newcode129
update_attempter.cc:129: if (!chrome_proxy_resolver_initialized_) {
On 2011/03/01 21:59:20, petkov wrote:
> does it make more sense to keep this flag in the ChromeProxyResolver class and
> check it in Init?

Done.

[petkov, 2011-03-02 05:46:40]

LGTM