Adds getter for ContextFactory to content

Adds getter for ContextFactory to content

Chrome (and others) need this so they can supply the ContextFactory to
the Compositor and get rid of Compositor::GetInstance.

BUG=none
TEST=none
R=piman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269447

[danakj, 2014-05-09 17:22:41]

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/g...
File content/browser/compositor/gpu_process_transport_factory.h (right):

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/g...
content/browser/compositor/gpu_process_transport_factory.h:52: virtual
ui::ContextFactory* AsContextFactory() OVERRIDE;
Can you just upcast this class instead?

[danakj, 2014-05-09 17:23:25]

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/g...
File content/browser/compositor/gpu_process_transport_factory.h (right):

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/g...
content/browser/compositor/gpu_process_transport_factory.h:52: virtual
ui::ContextFactory* AsContextFactory() OVERRIDE;
On 2014/05/09 17:22:42, danakj wrote:
> Can you just upcast this class instead?

Oh, nevermind, you're calling this on the ImageTransportFactory. I don't *think*
you can access this class directly there.

[sky, 2014-05-09 17:24:40]

On 2014/05/09 17:23:25, danakj wrote:
>
https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/g...
> File content/browser/compositor/gpu_process_transport_factory.h (right):
> 
>
https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/g...
> content/browser/compositor/gpu_process_transport_factory.h:52: virtual
> ui::ContextFactory* AsContextFactory() OVERRIDE;
> On 2014/05/09 17:22:42, danakj wrote:
> > Can you just upcast this class instead?
> 
> Oh, nevermind, you're calling this on the ImageTransportFactory. I don't
*think*
> you can access this class directly there.

I'm also just adding back code that was there. See
https://codereview.chromium.org/271843002 .

[danakj, 2014-05-09 17:36:53]

Yeh, I was glad to see it go is all. I guess the other option is to add a
ImageTransportFactory::ContextFactory() or some-such and a global pointer to the
context_factory that's set up in ImageTransportFactory::Initialize(). Either way
adds some public ImageTransportFactory API (one static and one virtual), so this
seems fine.

[sky, 2014-05-09 17:48:20]

It would seem that BrowserMainLoop should really own
ImageTransportFactory. That could be done later on though.

  -Scott

On Fri, May 9, 2014 at 10:36 AM,  <danakj@chromium.org> wrote:
> Yeh, I was glad to see it go is all. I guess the other option is to add a
> ImageTransportFactory::ContextFactory() or some-such and a global pointer to
> the
> context_factory that's set up in ImageTransportFactory::Initialize(). Either
> way
> adds some public ImageTransportFactory API (one static and one virtual), so
> this
> seems fine.
>
> https://codereview.chromium.org/276973002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2014-05-09 17:52:40]

lgtm

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/i...
File content/browser/compositor/image_transport_factory.h (right):

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/i...
content/browser/compositor/image_transport_factory.h:68: virtual
ui::ContextFactory* AsContextFactory() = 0;
nit: GetContextFactory? The NoTransport version is-not-a ContextFactory, but
has-a.

https://codereview.chromium.org/276973002/diff/1/content/public/browser/conte...
File content/public/browser/context_factory.h (right):

https://codereview.chromium.org/276973002/diff/1/content/public/browser/conte...
content/public/browser/context_factory.h:18: CONTENT_EXPORT ui::ContextFactory*
GetContextFactory();
nit: should this be in #if defined(USE_AURA), to make it cleat that it's not
available on any other platform?

[sky, 2014-05-09 18:04:52]

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/i...
File content/browser/compositor/image_transport_factory.h (right):

https://codereview.chromium.org/276973002/diff/1/content/browser/compositor/i...
content/browser/compositor/image_transport_factory.h:68: virtual
ui::ContextFactory* AsContextFactory() = 0;
On 2014/05/09 17:52:40, piman wrote:
> nit: GetContextFactory? The NoTransport version is-not-a ContextFactory, but
> has-a.

Ha. I nearly went with Get, but then I figured you had good reason for the As
before. I renamed it to GetContextFactory.

https://codereview.chromium.org/276973002/diff/1/content/public/browser/conte...
File content/public/browser/context_factory.h (right):

https://codereview.chromium.org/276973002/diff/1/content/public/browser/conte...
content/public/browser/context_factory.h:18: CONTENT_EXPORT ui::ContextFactory*
GetContextFactory();
On 2014/05/09 17:52:40, piman wrote:
> nit: should this be in #if defined(USE_AURA), to make it cleat that it's not
> available on any other platform?

I tend to think it should be up to callers to do the right thing. Similar to
other platform specific files.

[sky, 2014-05-09 18:04:57]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2014-05-09 18:05:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sky@chromium.org/276973002/20001

[commit-bot: I haz the power, 2014-05-09 21:46:12]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[sky, 2014-05-09 22:50:37]

Committed patchset #3 manually as r269447 (presubmit successful).