Stop having two URLLoaderFactories in RendererBlinkPlatformImpl

Stop having two URLLoaderFactories in RendererBlinkPlatformImpl

RendererBlinkPlatformImpl has one InterfacePtr<URLLoaderFactory> and one
AssociatedInterfacePtr<URLLoaderFactory> but only one instance is needed
at one time: They are used in the same way, but held differently because
of the type difference. This CL introduces a container type to hold them.

BUG=712913
R=kinuko@chromium.org, jam@chromium.org

Review-Url: https://codereview.chromium.org/2858503002
Cr-Commit-Position: refs/heads/master@{#470926}
Committed: https://chromium.googlesource.com/chromium/src/+/cecb6d2ec467966fcd43f0664f9820f526039765

[yhirano, 2017-05-02 07:56:49]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 07:57:07]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhirano, 2017-05-02 07:58:48]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 07:58:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kinuko, 2017-05-02 08:29:20]

https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
File content/common/possibly_associated_interface_ptr.h (right):

https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
content/common/possibly_associated_interface_ptr.h:6: #define
CONTENT_COMMON_POSSIBLY_ASSOCIATED_INTERFACE_PTR_H_
I imagine/expect this'd be a temporary thing, does this need to be in
content/common?

[yhirano, 2017-05-02 08:53:08]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 08:53:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhirano, 2017-05-02 08:53:40]

https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
File content/common/possibly_associated_interface_ptr.h (right):

https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
content/common/possibly_associated_interface_ptr.h:6: #define
CONTENT_COMMON_POSSIBLY_ASSOCIATED_INTERFACE_PTR_H_
On 2017/05/02 08:29:20, kinuko wrote:
> I imagine/expect this'd be a temporary thing, does this need to be in
> content/common?

Done.

[kinuko, 2017-05-02 09:50:41]

On 2017/05/02 08:53:40, yhirano (away May 3 - May 5) wrote:
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> File content/common/possibly_associated_interface_ptr.h (right):
> 
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> content/common/possibly_associated_interface_ptr.h:6: #define
> CONTENT_COMMON_POSSIBLY_ASSOCIATED_INTERFACE_PTR_H_
> On 2017/05/02 08:29:20, kinuko wrote:
> > I imagine/expect this'd be a temporary thing, does this need to be in
> > content/common?
> 
> Done.

I think my preference is to have this in renderer_blink_platform_impl.h and move
it out to a separate file when we actually start to use it elsewhere. (I assume
more change's coming)

[kinuko, 2017-05-02 09:53:49]

On 2017/05/02 09:50:41, kinuko wrote:
> On 2017/05/02 08:53:40, yhirano (away May 3 - May 5) wrote:
> >
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> > File content/common/possibly_associated_interface_ptr.h (right):
> > 
> >
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> > content/common/possibly_associated_interface_ptr.h:6: #define
> > CONTENT_COMMON_POSSIBLY_ASSOCIATED_INTERFACE_PTR_H_
> > On 2017/05/02 08:29:20, kinuko wrote:
> > > I imagine/expect this'd be a temporary thing, does this need to be in
> > > content/common?
> > 
> > Done.
> 
> I think my preference is to have this in renderer_blink_platform_impl.h and
move
> it out to a separate file when we actually start to use it elsewhere. (I
assume
> more change's coming)

...or we can wait John's comment to see if he thinks it's useful to have this
somewhere in more common place.

[commit-bot: I haz the power, 2017-05-02 09:54:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-02 09:54:19]

Dry run: This issue passed the CQ dry run.

[yhirano, 2017-05-02 10:05:46]

On 2017/05/02 09:53:49, kinuko wrote:
> On 2017/05/02 09:50:41, kinuko wrote:
> > On 2017/05/02 08:53:40, yhirano (away May 3 - May 5) wrote:
> > >
> >
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> > > File content/common/possibly_associated_interface_ptr.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> > > content/common/possibly_associated_interface_ptr.h:6: #define
> > > CONTENT_COMMON_POSSIBLY_ASSOCIATED_INTERFACE_PTR_H_
> > > On 2017/05/02 08:29:20, kinuko wrote:
> > > > I imagine/expect this'd be a temporary thing, does this need to be in
> > > > content/common?
> > > 
> > > Done.
> > 
> > I think my preference is to have this in renderer_blink_platform_impl.h and
> move
> > it out to a separate file when we actually start to use it elsewhere. (I
> assume
> > more change's coming)
> 
> ...or we can wait John's comment to see if he thinks it's useful to have this
> somewhere in more common place.

I will use it in RenderFrameImpl (or related classes).

[kinuko, 2017-05-02 10:13:31]

On 2017/05/02 10:05:46, yhirano (away May 3 - May 5) wrote:
> On 2017/05/02 09:53:49, kinuko wrote:
> > On 2017/05/02 09:50:41, kinuko wrote:
> > > On 2017/05/02 08:53:40, yhirano (away May 3 - May 5) wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> > > > File content/common/possibly_associated_interface_ptr.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2858503002/diff/20001/content/common/possibly...
> > > > content/common/possibly_associated_interface_ptr.h:6: #define
> > > > CONTENT_COMMON_POSSIBLY_ASSOCIATED_INTERFACE_PTR_H_
> > > > On 2017/05/02 08:29:20, kinuko wrote:
> > > > > I imagine/expect this'd be a temporary thing, does this need to be in
> > > > > content/common?
> > > > 
> > > > Done.
> > > 
> > > I think my preference is to have this in renderer_blink_platform_impl.h
and
> > move
> > > it out to a separate file when we actually start to use it elsewhere. (I
> > assume
> > > more change's coming)
> > 
> > ...or we can wait John's comment to see if he thinks it's useful to have
this
> > somewhere in more common place.
> 
> I will use it in RenderFrameImpl (or related classes).

I assumed you'd be using that, but in this particular CL having it in the same
file looks just enough.  You can easily move it in your next CL, am I right?

[jam, 2017-05-02 15:25:02]

lgtm

(content/renderer is fine, since that's where it's used now. if we need it in
browser/ or other content/ directories, then we can move it to a more general
place)

[kinuko, 2017-05-02 23:33:55]

lgtm/2

[kinuko, 2017-05-03 01:30:15]

On 2017/05/02 23:33:55, kinuko wrote:
> lgtm/2

(Just to be clear: I'm fine with this but I still prefer the general guideline,
i.e. put in the same file if it's not used anywhere else, then move it to more
appropriate place in the patch that starts to use it elsewhere-- it's easy/clear
to follow/review.)

[yhirano, 2017-05-11 07:13:55]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2017-05-11 07:13:56]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2017-05-11 09:39:26]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2017-05-11 09:40:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 11:59:08]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494495566429010,
"parent_rev": "8004ddbc047f7d41979a9faf3e076f275925b4fa", "commit_rev":
"cecb6d2ec467966fcd43f0664f9820f526039765"}

[commit-bot: I haz the power, 2017-05-11 11:59:21]

Description was changed from

==========
Stop having two URLLoaderFactories in RendererBlinkPlatformImpl

RendererBlinkPlatformImpl has one InterfacePtr<URLLoaderFactory> and one
AssociatedInterfacePtr<URLLoaderFactory> but only one instance is needed
at one time: They are used in the same way, but held differently because
of the type difference. This CL introduces a container type to hold them.

BUG=712913
R=kinuko@chromium.org, jam@chromium.org
==========

to

==========
Stop having two URLLoaderFactories in RendererBlinkPlatformImpl

RendererBlinkPlatformImpl has one InterfacePtr<URLLoaderFactory> and one
AssociatedInterfacePtr<URLLoaderFactory> but only one instance is needed
at one time: They are used in the same way, but held differently because
of the type difference. This CL introduces a container type to hold them.

BUG=712913
R=kinuko@chromium.org, jam@chromium.org

Review-Url: https://codereview.chromium.org/2858503002
Cr-Commit-Position: refs/heads/master@{#470926}
Committed:
https://chromium.googlesource.com/chromium/src/+/cecb6d2ec467966fcd43f0664f98...
==========

[commit-bot: I haz the power, 2017-05-11 11:59:22]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/cecb6d2ec467966fcd43f0664f98...