Fixes webrtc-internals stats reporting for the new stats API.

Fixes webrtc-internals stats reporting for the new stats API.

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

[jiayl, 2013-04-13 00:49:35]

PTAL.

[vrk (LEFT CHROMIUM), 2013-04-15 23:51:06]

LGTM

As discussed, please look to see if there's a way to check for the API mismatch
in the unit test, but let's check in the fix either way.

[jiayl, 2013-04-16 18:08:41]

The new patch added browser tests running real peer connection calls to verify
the browser APIs are expected. Also added warning comments in
peer_connection_tracker.

On 2013/04/15 23:51:06, Victoria Kirst wrote:
> LGTM
> 
> As discussed, please look to see if there's a way to check for the API
mismatch
> in the unit test, but let's check in the fix either way.

[vrk (LEFT CHROMIUM), 2013-04-17 00:40:59]

Cool! Thanks for finding a solution. The "retries" thing makes me worried about
flakiness, so please look into that before checking in.

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
File content/browser/media/webrtc_internals_browsertest.cc (right):

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
content/browser/media/webrtc_internals_browsertest.cc:473: EXPECT_TRUE(count >
1);
nit: Use EXPECT_GT here and below.

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
content/browser/media/webrtc_internals_browsertest.cc:486: while (count !=
NUMBER_OF_PEER_CONNECTIONS && retries < 10000) {
Hmmm... this "retries" thing is unideal. :) Can you talk to someone more
familiar with browser tests to see what's the convention for this?

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
content/browser/media/webrtc_internals_browsertest.cc:499:
ASSERT_TRUE(ExecuteScriptAndExtractBool(
nit: this block is indented 2 spaces too far

[jiayl, 2013-04-17 20:43:05]

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
File content/browser/media/webrtc_internals_browsertest.cc (right):

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
content/browser/media/webrtc_internals_browsertest.cc:473: EXPECT_TRUE(count >
1);
On 2013/04/17 00:40:59, Victoria Kirst wrote:
> nit: Use EXPECT_GT here and below.

Done.

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
content/browser/media/webrtc_internals_browsertest.cc:486: while (count !=
NUMBER_OF_PEER_CONNECTIONS && retries < 10000) {
The stats are reported asynchronously, so we need to wait for it. I used
'retries' to avoid waiting infinitely. But the browser test framework itself
already set a timeout limit on each test, so I remove the retries now.
On 2013/04/17 00:40:59, Victoria Kirst wrote:
> Hmmm... this "retries" thing is unideal. :) Can you talk to someone more
> familiar with browser tests to see what's the convention for this?

https://codereview.chromium.org/13859011/diff/3001/content/browser/media/webr...
content/browser/media/webrtc_internals_browsertest.cc:499:
ASSERT_TRUE(ExecuteScriptAndExtractBool(
On 2013/04/17 00:40:59, Victoria Kirst wrote:
> nit: this block is indented 2 spaces too far

Done.

[commit-bot: I haz the power, 2013-04-18 17:48:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jiayl@chromium.org/13859011/12001

[commit-bot: I haz the power, 2013-04-18 17:48:25]

Failed to apply patch for content/renderer/media/peer_connection_tracker.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/renderer/media/peer_connection_tracker.cc
  Hunk #2 FAILED at 191.
  1 out of 2 hunks FAILED -- saving rejects to file
content/renderer/media/peer_connection_tracker.cc.rej

Patch:       content/renderer/media/peer_connection_tracker.cc
Index: content/renderer/media/peer_connection_tracker.cc
diff --git a/content/renderer/media/peer_connection_tracker.cc
b/content/renderer/media/peer_connection_tracker.cc
index
203ba8b6202db4b046e4e51897ccb820d13d23f6..b3b1c33949bf3904405fe0e6acf87220e571e830
100644
--- a/content/renderer/media/peer_connection_tracker.cc
+++ b/content/renderer/media/peer_connection_tracker.cc
@@ -152,6 +152,9 @@ static string GetIceGatheringStateString(
 
 // Builds a DictionaryValue from the StatsReport.
 // The caller takes the ownership of the returned value.
+// Note:
+// The format must be consistent with what webrtc_internals.js expects.
+// If you change it here, you must change webrtc_internals.js as well.
 static DictionaryValue* GetDictValueStats(const webrtc::StatsReport& report) {
   if (report.values.empty())
     return NULL;
@@ -188,6 +191,9 @@ static DictionaryValue* GetDictValue(const
webrtc::StatsReport& report) {
   if (!result.get())
     return NULL;
 
+  // Note:
+  // The format must be consistent with what webrtc_internals.js expects.
+  // If you change it here, you must change webrtc_internals.js as well.
   if (stats.get())
     result->Set("stats", stats.release());
   result->SetString("id", report.id);

[commit-bot: I haz the power, 2013-04-18 17:57:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jiayl@chromium.org/13859011/17001

[commit-bot: I haz the power, 2013-04-18 21:46:30]

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

[commit-bot: I haz the power, 2013-04-18 21:49:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jiayl@chromium.org/13859011/17001

[commit-bot: I haz the power, 2013-04-19 03:07:27]

Change committed as 195058