Updated Chrome Test pages to use adapter.js

Updated Chrome Test pages to use adapter.js

BUG= Webrtc 1198
TEST= tested with Chrome and FireFox browser

[elham, 2013-07-11 18:39:09]

Hi,
Can you please review this

[DaleCurtis, 2013-07-11 20:26:13]

Is adapter.js something that can be checked in alongside these tests? It's
generally bad practice to depend on an external site, even if it's one we
control.

[phoglund_chromium, 2013-07-14 09:48:13]

On 2013/07/11 18:39:09, elham wrote:
> Hi,
> Can you please review this

Looks good in general. However, I've added a couple of new tests since you first
made this patch, so you'll have to rename getUserMedia -> doGetUserMedia in
those new tests as well. This search will show you all current places:
https://code.google.com/p/chromium/codesearch#search/&q=%5C%22getUserMedia%5C....

For instance,
chromium/src/chrome/browser/media/chrome_media_stream_infobar_browsertest.cc is
one such new test.

[elham, 2013-07-16 18:14:09]

Added 3 more files to review.

[kjellander_chromium, 2013-07-29 09:00:13]

+phoglund
Patrik, can you please help getting this done, as I'm still on parental leave?

[phoglund_chromium, 2013-07-29 11:27:55]

LGTM provided that the changes to webrtc_browser_test are rolled back (we can
fix that test in a separate CL).

https://codereview.chromium.org/18769010/diff/17001/content/browser/media/web...
File content/browser/media/webrtc_browsertest.cc (right):

https://codereview.chromium.org/18769010/diff/17001/content/browser/media/web...
content/browser/media/webrtc_browsertest.cc:55: 
Sorry, you should not be changing anything here since this calls to
getusermedia.html and not getusermedia.js (this is a content browser test, which
uses different HTML files than the ones you have been modifying). Just roll back
the changes in this file.

[DaleCurtis, 2013-07-29 17:09:19]

Hi can you please answer my Q above about adapter.js being hosted externally? 
If these tests are running on an automated bot, there shouldn't be external
dependencies like this.

[elham, 2013-07-29 21:07:35]

On 2013/07/29 17:09:19, DaleCurtis wrote:
> Hi can you please answer my Q above about adapter.js being hosted externally? 
> If these tests are running on an automated bot, there shouldn't be external
> dependencies like this.

Henrik/Patrik can help answer that question. I don't have a problem in adding
the adapter.js file if that's needed.

[elham, 2013-07-29 21:08:01]

On 2013/07/29 11:27:55, phoglund wrote:
> LGTM provided that the changes to webrtc_browser_test are rolled back (we can
> fix that test in a separate CL).
> 
>
https://codereview.chromium.org/18769010/diff/17001/content/browser/media/web...
> File content/browser/media/webrtc_browsertest.cc (right):
> 
>
https://codereview.chromium.org/18769010/diff/17001/content/browser/media/web...
> content/browser/media/webrtc_browsertest.cc:55: 
> Sorry, you should not be changing anything here since this calls to
> getusermedia.html and not getusermedia.js (this is a content browser test,
which
> uses different HTML files than the ones you have been modifying). Just roll
back
> the changes in this file.

Done

[phoglund_chromium, 2013-07-30 07:50:02]

+1 on Dale's suggestion.

[phoglund_chromium, 2013-07-30 08:46:10]

On 2013/07/30 07:50:02, phoglund wrote:
> +1 on Dale's suggestion.

Oh, you'll have to rebase and you'll get some conflicts because I moved all the
getUserMedia stuff in the browser tests to a single place in
https://codereview.chromium.org/20247003/. Sorry about that. The upside is that
all getUserMedia invocations happen in one place for all browser tests. Let me
know if you run into trouble; in that case I can take over the patch and land it
for you.

[elham, 2013-08-02 23:30:33]

On 2013/07/30 08:46:10, phoglund wrote:
> On 2013/07/30 07:50:02, phoglund wrote:
> > +1 on Dale's suggestion.
> 
> Oh, you'll have to rebase and you'll get some conflicts because I moved all
the
> getUserMedia stuff in the browser tests to a single place in
> https://codereview.chromium.org/20247003/. Sorry about that. The upside is
that
> all getUserMedia invocations happen in one place for all browser tests. Let me
> know if you run into trouble; in that case I can take over the patch and land
it
> for you.

Done

[elham, 2013-08-02 23:31:59]

Can you review again,  I have added the adapter.js and resolved the merge
conflict.

[phoglund_chromium, 2013-08-05 07:19:22]

On 2013/08/02 23:31:59, elham wrote:
> Can you review again,  I have added the adapter.js and resolved the merge
> conflict.

Oh wait, I think you misunderstood me earlier. I wanted you to roll back
webrtc_browsertest.cc, but you rolled back that AND the chrome_*_browsertest.cc
files. The chrome_*_browsertest files (such as chrome_webrtc_browsertest.cc)
need to be changed since they use getusermedia.js. webrtc_browsertest.cc,
however, does not. So please re-add the changes to those files (but NOT
webrtc_browsertest.cc).

[phoglund_chromium, 2013-08-06 06:56:29]

On 2013/08/05 07:19:22, phoglund wrote:
> On 2013/08/02 23:31:59, elham wrote:
> > Can you review again,  I have added the adapter.js and resolved the merge
> > conflict.
> 
> Oh wait, I think you misunderstood me earlier. I wanted you to roll back
> webrtc_browsertest.cc, but you rolled back that AND the
chrome_*_browsertest.cc
> files. The chrome_*_browsertest files (such as chrome_webrtc_browsertest.cc)
> need to be changed since they use getusermedia.js. webrtc_browsertest.cc,
> however, does not. So please re-add the changes to those files (but NOT
> webrtc_browsertest.cc).

Sorry, after https://codereview.chromium.org/20247003/ you only need to change
the call in webrtc_browsertest_base.cc. If you do that, you should be good to
commit.

[phoglund_chromium, 2013-08-06 07:01:29]

Dennis, we need a rubberstamp for the pyauto changes. This is a new version of
https://codereview.chromium.org/12824011/.

[dennis_jeffrey, 2013-08-06 16:44:00]

On 2013/08/06 07:01:29, phoglund wrote:
> Dennis, we need a rubberstamp for the pyauto changes. This is a new version of
> https://codereview.chromium.org/12824011/.

LGTM for chrome/test/functional/*

[DaleCurtis, 2013-08-06 17:48:08]

lgtm

[elham, 2013-08-06 18:00:38]

On 2013/08/06 06:56:29, phoglund wrote:
> On 2013/08/05 07:19:22, phoglund wrote:
> > On 2013/08/02 23:31:59, elham wrote:
> > > Can you review again,  I have added the adapter.js and resolved the merge
> > > conflict.
> > 
> > Oh wait, I think you misunderstood me earlier. I wanted you to roll back
> > webrtc_browsertest.cc, but you rolled back that AND the
> chrome_*_browsertest.cc
> > files. The chrome_*_browsertest files (such as chrome_webrtc_browsertest.cc)
> > need to be changed since they use getusermedia.js. webrtc_browsertest.cc,
> > however, does not. So please re-add the changes to those files (but NOT
> > webrtc_browsertest.cc).
> 
> Sorry, after https://codereview.chromium.org/20247003/ you only need to change
> the call in webrtc_browsertest_base.cc. If you do that, you should be good to
> commit.

Done

[phoglund1, 2013-08-07 07:59:37]

lgtm

[phoglund_chromium, 2013-08-07 08:00:11]

lgtm

[commit-bot: I haz the power, 2013-08-08 09:55:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/elham@google.com/18769010/57001

[commit-bot: I haz the power, 2013-08-08 09:55:36]

Failed to apply patch for chrome/browser/media/webrtc_browsertest_base.cc:
While running patch -p0 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/media/webrtc_browsertest_base.cc
  Hunk #1 FAILED at 124.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/media/webrtc_browsertest_base.cc.rej

Patch:       chrome/browser/media/webrtc_browsertest_base.cc
Index: chrome/browser/media/webrtc_browsertest_base.cc
===================================================================
--- chrome/browser/media/webrtc_browsertest_base.cc	(revision 215367)
+++ chrome/browser/media/webrtc_browsertest_base.cc	(working copy)
@@ -124,7 +124,7 @@
   // Request user media: this will launch the media stream info bar.
   EXPECT_EQ("ok-requested",
             ExecuteJavascript(
-                base::StringPrintf("getUserMedia(%s);", constraints.c_str()),
+                base::StringPrintf("doGetUserMedia(%s);", constraints.c_str()),
                 tab_contents));
 }

[phoglund_chromium, 2013-08-08 10:12:01]

You need to rebase the patch.

[commit-bot: I haz the power, 2013-08-09 18:31:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/elham@google.com/18769010/67001

[commit-bot: I haz the power, 2013-08-09 18:51:37]

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

[phoglund_chromium, 2013-08-15 08:10:22]

On 2013/08/09 18:51:37, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

Landed this as https://codereview.chromium.org/23172002/, let's hope it sticks.