Pass on download or installation errors from WebstoreInstaller to the CompleteInstall function.

Pass on download or installation errors from WebstoreInstaller to the CompleteInstall function.

This changes CompleteInstall from a SyncExtensionFunction that return as soon as the download is started to an AsyncExtensionFunction that returns only when an extension either finished installing or failed to install.

BUG=63213
BUG=140514
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=157618

[asargent_no_longer_on_chrome, 2012-09-10 23:52:10]

extensions parts LGTM - we need to make sure to give the webstore server-side
folks a heads up about this. (I'll send you the right email address to contact)

For reviewing the net/ change, I'd suggest trying willchan since he originally
began the deprecation of the older URLRequestInterceptor stuff, and will
probably know if there is some reason that export isn't already there and
whether you need to do something differently or not.

http://codereview.chromium.org/10911158/diff/1/chrome/browser/extensions/api/...
File chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
(right):

http://codereview.chromium.org/10911158/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc:124:
net::NetworkDelegate* network_delegate) OVERRIDE {
formatting nit: 

http://dev.chromium.org/developers/coding-style#TOC-Code-formatting

"For function declarations and definitions, put each argument on a separate line
unless everything fits on one line."

[Marijn Kruisselbrink, 2012-09-11 00:03:14]

willchan: is the change to url_request_job_manager correct?
URLRequest::Deprecated (which is exported) tells to use
URLRequestJobFactory::Interceptor for registering/unregistering of interceptors,
but that class/those methods are not exported, so I got linking errors when
trying to use them from my browsertest.

[willchan no longer on Chromium, 2012-09-11 00:24:03]

On 2012/09/11 00:03:14, Marijn Kruisselbrink wrote:
> willchan: is the change to url_request_job_manager correct?
> URLRequest::Deprecated (which is exported) tells to use
> URLRequestJobFactory::Interceptor for registering/unregistering of
interceptors,
> but that class/those methods are not exported, so I got linking errors when
> trying to use them from my browsertest.

I think you're confusing URLRequestJobManager and URLRequestJobFactory, right?
Are you trying to test a URLFetcher? Maybe you want to use TestURLFetcher or
testserver.py?

[Marijn Kruisselbrink, 2012-09-11 00:46:42]

On 2012/09/11 00:24:03, willchan wrote:
> On 2012/09/11 00:03:14, Marijn Kruisselbrink wrote:
> > willchan: is the change to url_request_job_manager correct?
> > URLRequest::Deprecated (which is exported) tells to use
> > URLRequestJobFactory::Interceptor for registering/unregistering of
> interceptors,
> > but that class/those methods are not exported, so I got linking errors when
> > trying to use them from my browsertest.
> 
> I think you're confusing URLRequestJobManager and URLRequestJobFactory, right?
> Are you trying to test a URLFetcher? Maybe you want to use TestURLFetcher or
> testserver.py?
Oh yes, sorry. You're right, I mixed up those classes. And actually thinking
more about it I should be able to get rid of the entire Interceptor bit since I
can just have the test serve up the right file to begin with. So never mind,
I'll go and fix this the proper way :)

[Marijn Kruisselbrink, 2012-09-13 16:49:51]

okay, did the test the proper and much simpler way eliminating the need for the
whole interceptor bit.

[asargent_no_longer_on_chrome, 2012-09-13 17:09:04]

Wow, that was much simpler. New version LGTM

[Mihai Parparita -not on Chrome, 2012-09-16 06:46:36]

LGTM

http://codereview.chromium.org/10911158/diff/3003/chrome/browser/extensions/a...
File chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
(right):

http://codereview.chromium.org/10911158/diff/3003/chrome/browser/extensions/a...
chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc:351:
ASSERT_TRUE(RunInstallTest("error204.html", "empty.crx"));
Does this actually result in a 204 status code? If not, the test name is a bit
of a misnomer.

[commit-bot: I haz the power, 2012-09-19 20:06:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mek@chromium.org/10911158/11001

[commit-bot: I haz the power, 2012-09-19 21:59:30]

Change committed as 157618