Change initialization interface of URLRequestJobFactory objects.

Change initialization interface of URLRequestJobFactory objects.

The associated CR involves moving mutation interfaces of URLRequestJobFactory to URLRequestJobFactory.  As such, their initializations need to maintain their type as URLRequestJobFactoryImpl before those interfaces can be removed.

See CL https://codereview.chromium.org/11227017 for more information.

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

[commit-bot: I haz the power, 2012-10-15 03:09:54]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[erikwright (departed), 2012-10-15 15:35:34]

If the associated CL is ready, it would be helpful to have a link to it here.

http://codereview.chromium.org/11154006/diff/1/chrome/browser/profiles/off_th...
File chrome/browser/profiles/off_the_record_profile_io_data.cc (right):

http://codereview.chromium.org/11154006/diff/1/chrome/browser/profiles/off_th...
chrome/browser/profiles/off_the_record_profile_io_data.cc:242:
net::URLRequestJobFactoryImpl* main_job_factory =
I think that assigning to a local scoped_ptr<URLRequestJobFactoryImpl> and then
.Pass()'ing to the member scoped_ptr<URLRequestJobFactory> (after line 266)
would be more correct.

http://codereview.chromium.org/11154006/diff/1/chrome/browser/profiles/off_th...
chrome/browser/profiles/off_the_record_profile_io_data.cc:300:
scoped_ptr<net::URLRequestJobFactory> job_factory(factory_impl);
You should be able to just change the template parameter of this local
scoped_ptr.

http://codereview.chromium.org/11154006/diff/1/chrome/browser/profiles/profil...
File chrome/browser/profiles/profile_impl_io_data.cc (right):

http://codereview.chromium.org/11154006/diff/1/chrome/browser/profiles/profil...
chrome/browser/profiles/profile_impl_io_data.cc:446:
net::URLRequestJobFactoryImpl* main_job_factory =
same comments as previous file.

http://codereview.chromium.org/11154006/diff/1/chrome/browser/profiles/profil...
chrome/browser/profiles/profile_impl_io_data.cc:557:
net::URLRequestJobFactoryImpl* factory_impl =
same comments as previous file.

[tedv, 2012-10-22 00:05:32]

Updated CL based on the feedback.

On 2012/10/15 15:35:34, erikwright wrote:
> If the associated CL is ready, it would be helpful to have a link to it here.

Forgive me for being a bit confused, but I thought this review page was the
source of the CL files.  Did you want a link to or from this page, and which
page is the source (or destination) page?

[erikwright (departed), 2012-10-22 01:20:15]

[+r mmenke, willchan->cc]

LGTM, mmkenke PTAL.

[erikwright (departed), 2012-10-22 01:22:51]

On 2012/10/22 00:05:32, tedv wrote:
> Updated CL based on the feedback.
> 
> On 2012/10/15 15:35:34, erikwright wrote:
> > If the associated CL is ready, it would be helpful to have a link to it
here.
> 
> Forgive me for being a bit confused, but I thought this review page was the
> source of the CL files.  Did you want a link to or from this page, and which
> page is the source (or destination) page?

The description says "The associated CR involves moving mutation interfaces of
URLRequestJobFactory to URLRequestJobFactory". I interpreted this as meaning
that, in a follow-up CL, you were pulling mutation methods from
URLRequestJobFactory down to URLRequestJobFactoryImpl.

If that CL is already prepared, I suggest providing a link to it now in order to
provide some context to this CL.

[tedv, 2012-10-22 03:45:32]

On 2012/10/22 01:22:51, erikwright wrote:
> If that CL is already prepared, I suggest providing a link to it now in order
to
> provide some context to this CL.

That makes more sense.  I added it to the main CL description.

[mmenke, 2012-10-22 14:42:51]

http://codereview.chromium.org/11154006/diff/14002/chrome/browser/profiles/pr...
File chrome/browser/profiles/profile_io_data.h (right):

http://codereview.chromium.org/11154006/diff/14002/chrome/browser/profiles/pr...
chrome/browser/profiles/profile_io_data.h:23: #include
"net/url_request/url_request_job_factory_impl.h"
You should forward declare UrlRequestJobFactory and just include
url_request_job_factory here.  Reminder:  Forward declarations are in
alphabetical order.

[mmenke, 2012-10-24 15:57:05]

LGTM, thanks!  You should generally post a comment when responding to reviews,
or the changes may go overlooked.

[tedv, 2012-10-24 16:16:17]

On 2012/10/24 15:57:05, Matt Menke wrote:
> LGTM, thanks!  You should generally post a comment when responding to reviews,
> or the changes may go overlooked.

I usually do, but forgot this time.  I'll chalk it up to being late at night.

[commit-bot: I haz the power, 2012-10-24 16:17:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedv@chromium.org/11154006/21001

[commit-bot: I haz the power, 2012-10-24 16:37:15]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
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.

[mmenke, 2012-10-24 16:50:29]

On 2012/10/24 16:16:17, tedv wrote:
> On 2012/10/24 15:57:05, Matt Menke wrote:
> > LGTM, thanks!  You should generally post a comment when responding to
reviews,
> > or the changes may go overlooked.
> 
> I usually do, but forgot this time.  I'll chalk it up to being late at night.

No need for another round of review - just add the missing header to
profile_impl_io_data.cc (In alphabetical order, of course), and you're good to
reland.

[tedv, 2012-10-24 17:10:52]

On 2012/10/24 16:50:29, Matt Menke wrote:
> No need for another round of review - just add the missing header to
> profile_impl_io_data.cc (In alphabetical order, of course), and you're good to
> reland.

Yeah, I've had my share of rejected commits before.  I'll get this patched up
when I get home this evening.

[mmenke, 2012-10-24 17:11:47]

On 2012/10/24 17:10:52, tedv wrote:
> On 2012/10/24 16:50:29, Matt Menke wrote:
> > No need for another round of review - just add the missing header to
> > profile_impl_io_data.cc (In alphabetical order, of course), and you're good
to
> > reland.
> 
> Yeah, I've had my share of rejected commits before.  I'll get this patched up
> when I get home this evening.

Sorry - I had assumed you were a very new committer.

[commit-bot: I haz the power, 2012-10-24 23:14:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedv@chromium.org/11154006/30003

[commit-bot: I haz the power, 2012-10-25 01:19:08]

Change committed as 163978