Issue 9433006:
Remove GetAudioManager and GetMediaStreamManager from ResourceContext. The reason is the content mo… (Closed)
Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: fix deps
Created 8 years, 10 months ago
Use n/p to move between diff chunks;
N/P to move between comments.
Draft comments are only viewable by you.
tommi (sloooow) - chröme
2012/02/21 21:04:08
See the comment above for g_audio_manager:
" // Us
See the comment above for g_audio_manager:
" // Used only to make sure we never create more than one instance."
I intend to remove this variable, but if we do this, we start to actually rely
on it again which would be a step backwards.
Fixing this for content_client has actually been on the TODO list though (so
thanks for doing this!) - but what I was planning on doing was to add instead an
audio_manager() property to ContentClient and access it from there. WDYT?
jam
2012/02/21 21:09:47
got it. I didn't know you were planning on removin
On 2012/02/21 21:04:08, tommi wrote:
> See the comment above for g_audio_manager:
> " // Used only to make sure we never create more than one instance."
>
> I intend to remove this variable, but if we do this, we start to actually rely
> on it again which would be a step backwards.
got it. I didn't know you were planning on removing it, and so I didn't want to
duplicate this global in both media and content.
I'll remove it from media and move it to content.
>
> Fixing this for content_client has actually been on the TODO list though (so
> thanks for doing this!) - but what I was planning on doing was to add instead
an
> audio_manager() property to ContentClient and access it from there. WDYT?
Do you mean ContentBrowserClient (ContentClient is an interface that embedders
implement in each process, while CBC is one that's implemented in the browser
process)? If so, then that would take us back to the embedder owning and
creating this object, which is what this change is trying to avoid. I'll look
for a good place to store this in content (usually we store it with the class,
but in this case the class is outside of content)
tommi (sloooow) - chröme
2012/02/21 21:15:23
Yes, ContentBrowserClient.
On 2012/02/21 21:09:47, John Abd-El-Malek wrote:
> On 2012/02/21 21:04:08, tommi wrote:
> > See the comment above for g_audio_manager:
> > " // Used only to make sure we never create more than one instance."
> >
> > I intend to remove this variable, but if we do this, we start to actually
rely
> > on it again which would be a step backwards.
>
> got it. I didn't know you were planning on removing it, and so I didn't want
to
> duplicate this global in both media and content.
>
> I'll remove it from media and move it to content.
>
> >
> > Fixing this for content_client has actually been on the TODO list though (so
> > thanks for doing this!) - but what I was planning on doing was to add
instead
> an
> > audio_manager() property to ContentClient and access it from there. WDYT?
>
> Do you mean ContentBrowserClient (ContentClient is an interface that embedders
> implement in each process, while CBC is one that's implemented in the browser
> process)?
> If so, then that would take us back to the embedder owning and
> creating this object, which is what this change is trying to avoid. I'll look
> for a good place to store this in content (usually we store it with the class,
> but in this case the class is outside of content)
Issue 9433006: Remove GetAudioManager and GetMediaStreamManager from ResourceContext. The reason is the content mo…
(Closed)
Created 8 years, 10 months ago by jam
Modified 8 years, 10 months ago
Reviewers: scherkus (not reviewing), tommi (sloooow) - chröme, vrk (LEFT CHROMIUM)
Base URL: svn://chrome-svn/chrome/trunk/src/
Comments: 14