Ensure that WebRTC AEC recording checkbox is not set if cancelling file dialog.

Ensure that WebRTC AEC recording checkbox is not set if cancelling file dialog.

NOTRY=true

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

[Ami GONE FROM CHROMIUM, 2014-03-27 19:04:53]

LGTM % nits

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_internals.cc (right):

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_internals.cc:293: #if defined(ENABLE_WEBRTC)
lol why is this file even being compiled if !ENABLE_WEBRTC?!?

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_internals_ui_observer.h (right):

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_internals_ui_observer.h:21: // This is called on
the browser IO thread. |args| can be NULL if there's no
there's -> there're
(or, "there are"; the plural of "arguments" means the singular "is" is
incorrect)

https://codereview.chromium.org/213703004/diff/1/content/browser/resources/me...
File content/browser/resources/media/dump_creator.js (right):

https://codereview.chromium.org/213703004/diff/1/content/browser/resources/me...
content/browser/resources/media/dump_creator.js:63: /** Mark the AEC recording
checkbox unchecked.*/
Why /** instead of /*, or, more likely, just // ?

(/** means jsdoc AFAIK, but you don't really want this or l.58 being jsdoc'd, do
you?)

https://codereview.chromium.org/213703004/diff/1/content/browser/resources/me...
content/browser/resources/media/dump_creator.js:64:
aecRecordingFileSelectionCancelled: function() {
not just "disableAecRecording"?

[Henrik Grunell, 2014-03-28 09:06:00]

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_internals.cc (right):

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_internals.cc:293: #if defined(ENABLE_WEBRTC)
On 2014/03/27 19:04:53, Ami Fischman wrote:
> lol why is this file even being compiled if !ENABLE_WEBRTC?!?

:) It's indeed more or less hilarious. I noticed this when adding the AEC dump
to webrtc-internals and added a todo at the first occurence of ifdef (in the
constructor). It's hacky and should be fixed. Filed http://crbug.com/357517.

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
File content/browser/media/webrtc_internals_ui_observer.h (right):

https://codereview.chromium.org/213703004/diff/1/content/browser/media/webrtc...
content/browser/media/webrtc_internals_ui_observer.h:21: // This is called on
the browser IO thread. |args| can be NULL if there's no
On 2014/03/27 19:04:53, Ami Fischman wrote:
> there's -> there're
> (or, "there are"; the plural of "arguments" means the singular "is" is
> incorrect)

Done.

https://codereview.chromium.org/213703004/diff/1/content/browser/resources/me...
File content/browser/resources/media/dump_creator.js (right):

https://codereview.chromium.org/213703004/diff/1/content/browser/resources/me...
content/browser/resources/media/dump_creator.js:63: /** Mark the AEC recording
checkbox unchecked.*/
On 2014/03/27 19:04:53, Ami Fischman wrote:
> Why /** instead of /*, or, more likely, just // ?
> 
> (/** means jsdoc AFAIK, but you don't really want this or l.58 being jsdoc'd,
do
> you?)

Agree, done.

https://codereview.chromium.org/213703004/diff/1/content/browser/resources/me...
content/browser/resources/media/dump_creator.js:64:
aecRecordingFileSelectionCancelled: function() {
On 2014/03/27 19:04:53, Ami Fischman wrote:
> not just "disableAecRecording"?

Agree, done.

[Henrik Grunell, 2014-03-28 09:06:08]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-03-28 09:08:05]

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

[Henrik Grunell, 2014-03-28 11:20:48]

The CQ bit was unchecked by grunell@chromium.org

[Henrik Grunell, 2014-03-28 11:20:54]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-03-28 13:52:46]

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

[commit-bot: I haz the power, 2014-03-28 13:52:46]

Retried try job too often on ios_dbg_simulator for step(s) url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[Henrik Grunell, 2014-03-28 16:41:53]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-03-28 16:45:49]

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

[commit-bot: I haz the power, 2014-03-28 16:54:10]

Change committed as 260168