fix browser crash when closing 2 or more tabs running getUserMedia

We should not delete the iter if iter == tabs_.end()


BUG=144318
TEST=manual test:
1) Three tabs, each pointing to a page that makes a getUserMedia request. For some reason, I can't seem to repro this bug with just two tabs.
2) Click Allow in the infobar of each tab
3) Right click any of those tabs and select "Close other tabs"

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

[no longer working on chromium, 2012-08-24 12:20:05]

Hi Ami, could you please take a quick look at this fix?

[Ami GONE FROM CHROMIUM, 2012-08-24 17:57:57]

https://chromiumcodereview.appspot.com/10867049/diff/1/chrome/browser/media/m...
File chrome/browser/media/media_stream_capture_indicator.cc (right):

https://chromiumcodereview.appspot.com/10867049/diff/1/chrome/browser/media/m...
chrome/browser/media/media_stream_capture_indicator.cc:429: if (iter !=
tabs_.end()) {
Do you understand how it can happen that this is ==?

https://chromiumcodereview.appspot.com/10867049/diff/1/chrome/browser/media/m...
chrome/browser/media/media_stream_capture_indicator.cc:448:
UpdateStatusTrayIconContextMenu();
Do you want to do this even if iter==tabs_.end() (and thus no other work has
been done)?
If not, can replace the != with a == and early-return, and de-indent l.430-445.

[no longer working on chromium, 2012-08-24 18:09:58]

http://codereview.chromium.org/10867049/diff/1/chrome/browser/media/media_str...
File chrome/browser/media/media_stream_capture_indicator.cc (right):

http://codereview.chromium.org/10867049/diff/1/chrome/browser/media/media_str...
chrome/browser/media/media_stream_capture_indicator.cc:429: if (iter !=
tabs_.end()) {
On 2012/08/24 17:57:57, Ami Fischman wrote:
> Do you understand how it can happen that this is ==?

Yes. This happens when we have multiple tabs using getUserMedia, and we "close
other tabs".
When tabs go away at the same time, in UpdateStatusTrayIconContextMenu, we
delete the element if we find out the tab has gone away, so the next
notification will hit this.

http://codereview.chromium.org/10867049/diff/1/chrome/browser/media/media_str...
chrome/browser/media/media_stream_capture_indicator.cc:448:
UpdateStatusTrayIconContextMenu();
On 2012/08/24 17:57:57, Ami Fischman wrote:
> Do you want to do this even if iter==tabs_.end() (and thus no other work has
> been done)?
> If not, can replace the != with a == and early-return, and de-indent
l.430-445.

Good question, I think both ways work the same. But I did it this way since I
felt it was safer to call  UpdateStatusTrayIconContextMenu here.
But I am quite OK to switch to another if you think it is better.

[Ami GONE FROM CHROMIUM, 2012-08-24 18:21:41]

LGTM

http://codereview.chromium.org/10867049/diff/1/chrome/browser/media/media_str...
File chrome/browser/media/media_stream_capture_indicator.cc (right):

http://codereview.chromium.org/10867049/diff/1/chrome/browser/media/media_str...
chrome/browser/media/media_stream_capture_indicator.cc:448:
UpdateStatusTrayIconContextMenu();
On 2012/08/24 18:09:58, xians1 wrote:
> On 2012/08/24 17:57:57, Ami Fischman wrote:
> > Do you want to do this even if iter==tabs_.end() (and thus no other work has
> > been done)?
> > If not, can replace the != with a == and early-return, and de-indent
> l.430-445.
> 
> Good question, I think both ways work the same. But I did it this way since I
> felt it was safer to call  UpdateStatusTrayIconContextMenu here.
> But I am quite OK to switch to another if you think it is better.
> 

I don't know the code to make a call.  It's up to you.

[no longer working on chromium, 2012-08-27 09:18:44]

> 
> I don't know the code to make a call.  It's up to you.

We once had an issue that we leaked the system tray UI while the tabs went away.
So I am not confident on relying on the notifications from MediaStream. 
I still feel the current approach is safer, and am going to land this soon.

[commit-bot: I haz the power, 2012-08-27 09:19:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/10867049/1

[commit-bot: I haz the power, 2012-08-27 12:06:18]

Change committed as 153465