Change MediaStreamCaptureIndicator back to ref counted

Change MediaStreamCaptureIndicator back to ref counted

This should fix a DCHECK (and potential bug) introduced in https://chromiumcodereview.appspot.com/11236052/

BUG=157986
For UI changes from original CL:
TBR=sky@chromium.org



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

[stevenjb, 2012-10-26 00:40:50]

Sorry for introducing this. Building a Debug version now, will test it first
thing in the morning.

[no longer working on chromium, 2012-10-26 08:11:20]

Thanks for the quick action. Some comments about the scoped_refptr, please take
a look.

BR,
SX

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
File chrome/browser/media/media_internals.cc (right):

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
chrome/browser/media/media_internals.cc:72:
GetMediaStreamCaptureIndicator()->CaptureDevicesOpened(render_process_id,
MeidaInternals owns the media_stream_capture_indicator_, why not simply call
media_stream_capture_indicator_ here

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
chrome/browser/media/media_internals.cc:82:
GetMediaStreamCaptureIndicator()->CaptureDevicesClosed(render_process_id,
ditto

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
chrome/browser/media/media_internals.cc:114:
scoped_refptr<MediaStreamCaptureIndicator>
I think we don't need to return a scoped_refptr, clients get access to
media_stream_capture_indicator_ via MediaInternals which should go away before
MediaInternals, otherwise it will crash anyhow when trying to access
MediaInternals.
We can simply return a normal pointer here.

[stevenjb, 2012-10-26 20:35:08]

OK, merged in the original changes which got reverted and addressed your
comments.
PTAL

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
File chrome/browser/media/media_internals.cc (right):

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
chrome/browser/media/media_internals.cc:72:
GetMediaStreamCaptureIndicator()->CaptureDevicesOpened(render_process_id,
On 2012/10/26 08:11:20, xians1 wrote:
> MeidaInternals owns the media_stream_capture_indicator_, why not simply call
> media_stream_capture_indicator_ here
Yes, right, thanks. Done.

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
chrome/browser/media/media_internals.cc:82:
GetMediaStreamCaptureIndicator()->CaptureDevicesClosed(render_process_id,
On 2012/10/26 08:11:20, xians1 wrote:
> ditto

Done.

http://codereview.chromium.org/11273060/diff/1/chrome/browser/media/media_int...
chrome/browser/media/media_internals.cc:114:
scoped_refptr<MediaStreamCaptureIndicator>
On 2012/10/26 08:11:20, xians1 wrote:
> I think we don't need to return a scoped_refptr, clients get access to
> media_stream_capture_indicator_ via MediaInternals which should go away before
> MediaInternals, otherwise it will crash anyhow when trying to access
> MediaInternals.
> We can simply return a normal pointer here.
At the moment that is true. However, since this is a public accessor, I would
rather do this safely and remind the consumer (who will hopefully pay more
attention than I did) that they are dealing with a class that may get updated on
another thread.

[no longer working on chromium, 2012-10-26 21:14:51]

I did not  go through the whole CL but only the changes about ref-count, lgtm if
the rest of the changes are from the old CL.

[commit-bot: I haz the power, 2012-10-26 21:41:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/11273060/5001

[commit-bot: I haz the power, 2012-10-26 21:41:13]

Presubmit check for 11273060-5001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Messages **
--tbr was specified, skipping OWNERS check

** Presubmit ERRORS **
Image chrome/app/theme/default_200_percent/tab_recording.png has width 40,
expected to be 32

Image chrome/app/theme/default_200_percent/tab_recording.png has height 40,
expected to be 32

[commit-bot: I haz the power, 2012-10-26 22:58:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/11273060/13001

[commit-bot: I haz the power, 2012-10-27 05:57:57]

Change committed as 164508