Adding WebRTC log upload list under chrome://webrtc-logs

Adding WebRTC log upload list under chrome://webrtc-logs

CL for writing to the file that lists the uploaded logs:
https://codereview.chromium.org/17589014/

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

[Henrik Grunell, 2013-06-20 15:12:29]

James, please have a look at this. It's a similar page as chrome://crashes.

Besides general reviewing, I get an error now, "This page is unavailable" with
error code ERR_FAILED. Maybe you can see right away what's missing. There's also
details to fix once I can view the page, e.g. the css file.

[Henrik Grunell, 2013-06-24 11:44:25]

On 2013/06/20 15:12:29, Henrik Grunell wrote:
> James, please have a look at this. It's a similar page as chrome://crashes.
> 
> Besides general reviewing, I get an error now, "This page is unavailable" with
> error code ERR_FAILED. Maybe you can see right away what's missing. There's
also
> details to fix once I can view the page, e.g. the css file.

I've fixed the problem. Please review this.

[James Hawkins, 2013-06-24 15:05:07]

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
File chrome/browser/media/webrtc_log_upload_list.h (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:10: class WebRtcLogUploadList :
public UploadList {
nit: Document class.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:12: // Static factory method that
creates the WebRTC log upload list with the
Optional nit: "Static factory method that" is superfluous.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:16: static const char*
kWebRtcLogListFilename;
Why is this public?  Documentation would help.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:20: const base::FilePath&
upload_log_path);
nit: Document |upload_log_path|.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
File chrome/browser/resources/media/webrtc_logs.html (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
chrome/browser/resources/media/webrtc_logs.html:15: <h2 id="logBanner"></h2>
nit: IDs should be dash-form.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
File chrome/browser/resources/media/webrtc_logs.js (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
chrome/browser/resources/media/webrtc_logs.js:18: function
updateWebRtcLogsList(enabled, uploads, version) {
Where is this used?

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/ui/w...
File chrome/browser/ui/webui/media/webrtc_logs_ui.h (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/media/webrtc_logs_ui.h:15: class WebRtcLogsUI : public
content::WebUIController {
nit: Document class.

[Henrik Grunell, 2013-06-24 17:11:44]

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
File chrome/browser/media/webrtc_log_upload_list.h (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:10: class WebRtcLogUploadList :
public UploadList {
On 2013/06/24 15:05:07, James Hawkins wrote:
> nit: Document class.

Done.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:12: // Static factory method that
creates the WebRTC log upload list with the
On 2013/06/24 15:05:07, James Hawkins wrote:
> Optional nit: "Static factory method that" is superfluous.

Done.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:16: static const char*
kWebRtcLogListFilename;
On 2013/06/24 15:05:07, James Hawkins wrote:
> Why is this public?  Documentation would help.

Done.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/medi...
chrome/browser/media/webrtc_log_upload_list.h:20: const base::FilePath&
upload_log_path);
On 2013/06/24 15:05:07, James Hawkins wrote:
> nit: Document |upload_log_path|.

Done.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
File chrome/browser/resources/media/webrtc_logs.html (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
chrome/browser/resources/media/webrtc_logs.html:15: <h2 id="logBanner"></h2>
On 2013/06/24 15:05:07, James Hawkins wrote:
> nit: IDs should be dash-form.

Done.

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
File chrome/browser/resources/media/webrtc_logs.js (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/reso...
chrome/browser/resources/media/webrtc_logs.js:18: function
updateWebRtcLogsList(enabled, uploads, version) {
On 2013/06/24 15:05:07, James Hawkins wrote:
> Where is this used?

In WebRtcLogsDOMHandler::UpdateUI (webrtc_logs_ui.cc).

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/ui/w...
File chrome/browser/ui/webui/media/webrtc_logs_ui.h (right):

https://chromiumcodereview.appspot.com/17063004/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/media/webrtc_logs_ui.h:15: class WebRtcLogsUI : public
content::WebUIController {
On 2013/06/24 15:05:07, James Hawkins wrote:
> nit: Document class.

Done.

[James Hawkins, 2013-06-24 18:31:06]

https://chromiumcodereview.appspot.com/17063004/diff/13001/chrome/browser/med...
File chrome/browser/media/webrtc_log_upload_list.h (right):

https://chromiumcodereview.appspot.com/17063004/diff/13001/chrome/browser/med...
chrome/browser/media/webrtc_log_upload_list.h:18: static const char*
kWebRtcLogListFilename;
I still don't understand why this member var is public.  Is it WebRtcLogUploader
that needs it outside of this class?  This seems smelly, but my nose is stuffed.

[Henrik Grunell, 2013-06-24 20:29:34]

https://chromiumcodereview.appspot.com/17063004/diff/13001/chrome/browser/med...
File chrome/browser/media/webrtc_log_upload_list.h (right):

https://chromiumcodereview.appspot.com/17063004/diff/13001/chrome/browser/med...
chrome/browser/media/webrtc_log_upload_list.h:18: static const char*
kWebRtcLogListFilename;
On 2013/06/24 18:31:06, James Hawkins wrote:
> I still don't understand why this member var is public.  Is it
WebRtcLogUploader
> that needs it outside of this class?  This seems smelly, but my nose is
stuffed.

Yes that's correct. I did it the same way as for chrome://crashes, see
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/crash_upload_l...

[James Hawkins, 2013-06-26 15:18:53]

LGTM with nit.

https://chromiumcodereview.appspot.com/17063004/diff/20001/chrome/browser/ui/...
File chrome/browser/ui/webui/media/webrtc_logs_ui.cc (right):

https://chromiumcodereview.appspot.com/17063004/diff/20001/chrome/browser/ui/...
chrome/browser/ui/webui/media/webrtc_logs_ui.cc:92:
scoped_refptr<WebRtcLogUploadList> upload_list_;
nit: Document member variables.

[Henrik Grunell, 2013-06-27 08:16:34]

https://chromiumcodereview.appspot.com/17063004/diff/20001/chrome/browser/ui/...
File chrome/browser/ui/webui/media/webrtc_logs_ui.cc (right):

https://chromiumcodereview.appspot.com/17063004/diff/20001/chrome/browser/ui/...
chrome/browser/ui/webui/media/webrtc_logs_ui.cc:92:
scoped_refptr<WebRtcLogUploadList> upload_list_;
On 2013/06/26 15:18:53, James Hawkins wrote:
> nit: Document member variables.

Done.

[commit-bot: I haz the power, 2013-06-27 08:28:19]

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

[commit-bot: I haz the power, 2013-06-27 14:17:07]

Change committed as 208910