Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

BUG=589161
Committed: https://crrev.com/0685703cc76e6a84d49d1d3033ae7b20ba66a3ae
Cr-Commit-Position: refs/heads/master@{#377642}

[Guido Urdaneta, 2016-02-23 18:31:54]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-23 18:36:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1729563002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1729563002/1

[commit-bot: I haz the power, 2016-02-23 18:53:31]

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

[commit-bot: I haz the power, 2016-02-23 18:53:31]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Guido Urdaneta, 2016-02-24 07:11:03]

Description was changed from

==========
Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

BUG=589161
==========

to

==========
Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

BUG=589161
==========

[Guido Urdaneta, 2016-02-24 07:11:04]

guidou@chromium.org changed reviewers:
+ hta@chromium.org, philipj@opera.com

[Guido Urdaneta, 2016-02-24 07:13:57]

Hi, PTAL.
After the review of the other CL I was thinking that if DOMExceptions convert to
strings, perhaps it's not that problematic to replace RTCErrorCallback with
RTCPeerConnectionCallback. WDYT?

[philipj_slow, 2016-02-24 08:14:03]

lgtm % test nits, and I'm not sure about the ActiveDOMObject change.

There are some failing tests in Chromium as well, it seems.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html
(right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html:18:
function requestFailed2(reason)
s/reason/error/?

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html:23:
shouldBe('error.name', '"OperationError"');
Also check error.message, or perhaps error.toString() to prove that treating the
new value as a string will work fine.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-ice.html
(right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-ice.html:34:
function expectedTypeError(reason)
I think error was a better name :)

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-ice.html:63:
shouldBe('error.name', '"InvalidStateError"');
It wouldn't hurt to check error.message as well, in case there will be multiple
ways of getting a particular type of exception.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-localDescription.html
(right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-localDescription.html:48:
window.error = reason;
Oops, here the local variable is still called error, why does this test still
pass? Suggest s/reason/error/ everywhere and won't comment elsewhere.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-localDescription.html:61:
localPeerConnection.setLocalDescription(sessionDescription, unexpectedCallback,
(reason) => {
Just as an FYI, it's OK to omit the parens when there's only one argument for an
arrow function. Doesn't matter, though.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.h (right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.h:52: class
RTCErrorCallback;
Remove this forward declaration, grep source for any other remaining bits?

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/mediastream/RTCSessionDescriptionRequestImpl.h
(left):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/RTCSessionDescriptionRequestImpl.h:47:
class RTCSessionDescriptionRequestImpl final : public
RTCSessionDescriptionRequest, public ActiveDOMObject {
Is the ActiveDOMObject change made possible by this refactoring, or is it a
drive-by fix?

[Guido Urdaneta, 2016-02-24 12:42:10]

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html
(right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html:18:
function requestFailed2(reason)
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> s/reason/error/?

Done.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html:23:
shouldBe('error.name', '"OperationError"');
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> Also check error.message, or perhaps error.toString() to prove that treating
the
> new value as a string will work fine.

Done.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-ice.html
(right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-ice.html:34:
function expectedTypeError(reason)
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> I think error was a better name :)

Done.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-ice.html:63:
shouldBe('error.name', '"InvalidStateError"');
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> It wouldn't hurt to check error.message as well, in case there will be
multiple
> ways of getting a particular type of exception.

Done.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-localDescription.html
(right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-localDescription.html:48:
window.error = reason;
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> Oops, here the local variable is still called error, why does this test still
> pass? Suggest s/reason/error/ everywhere and won't comment elsewhere.

Done.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/RTCPeerConnection-localDescription.html:61:
localPeerConnection.setLocalDescription(sessionDescription, unexpectedCallback,
(reason) => {
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> Just as an FYI, it's OK to omit the parens when there's only one argument for
an
> arrow function. Doesn't matter, though.

Done.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.h (right):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.h:52: class
RTCErrorCallback;
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> Remove this forward declaration, grep source for any other remaining bits?

Done.

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/mediastream/RTCSessionDescriptionRequestImpl.h
(left):

https://codereview.chromium.org/1729563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/RTCSessionDescriptionRequestImpl.h:47:
class RTCSessionDescriptionRequestImpl final : public
RTCSessionDescriptionRequest, public ActiveDOMObject {
On 2016/02/24 08:14:03, philipj_UTC7 wrote:
> Is the ActiveDOMObject change made possible by this refactoring, or is it a
> drive-by fix?

Reverted.

[Guido Urdaneta, 2016-02-24 13:26:21]

guidou@chromium.org changed reviewers:
+ jochen@chromium.org

[Guido Urdaneta, 2016-02-24 13:26:21]

+jochen

[Guido Urdaneta, 2016-02-24 16:24:45]

guidou@chromium.org changed reviewers:
+ tommi@chromium.org
- jochen@chromium.org

[Guido Urdaneta, 2016-02-24 16:26:01]

+tommi: can you rs?

[Guido Urdaneta, 2016-02-24 17:15:08]

guidou@chromium.org changed reviewers:
+ jochen@chromium.org

[Guido Urdaneta, 2016-02-25 10:42:15]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-25 10:43:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1729563002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1729563002/80001

[tommi (sloooow) - chröme, 2016-02-25 11:30:30]

lgtm

[jochen (gone - plz use gerrit), 2016-02-25 12:27:46]

lgtm

[commit-bot: I haz the power, 2016-02-25 14:12:53]

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

[commit-bot: I haz the power, 2016-02-25 14:12:54]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Guido Urdaneta, 2016-02-25 14:14:33]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2016-02-25 14:14:34]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1729563002/#ps80001
(title: "Move TODO comment")

[commit-bot: I haz the power, 2016-02-25 14:14:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1729563002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1729563002/80001

[commit-bot: I haz the power, 2016-02-25 17:41:47]

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

[commit-bot: I haz the power, 2016-02-25 17:41:52]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Guido Urdaneta, 2016-02-25 17:59:14]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2016-02-25 17:59:15]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
tommi@chromium.org, philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1729563002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-02-25 17:59:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1729563002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1729563002/100001

[commit-bot: I haz the power, 2016-02-25 19:29:47]

Description was changed from

==========
Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

BUG=589161
==========

to

==========
Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

BUG=589161
==========

[commit-bot: I haz the power, 2016-02-25 19:29:48]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-02-25 19:30:49]

Description was changed from

==========
Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

BUG=589161
==========

to

==========
Replace RTCErrorCallback with RTCPeerConnectionErrorCallback.

BUG=589161

Committed: https://crrev.com/0685703cc76e6a84d49d1d3033ae7b20ba66a3ae
Cr-Commit-Position: refs/heads/master@{#377642}
==========

[commit-bot: I haz the power, 2016-02-25 19:30:50]

Patchset 6 (id:??) landed as
https://crrev.com/0685703cc76e6a84d49d1d3033ae7b20ba66a3ae
Cr-Commit-Position: refs/heads/master@{#377642}

[Guido Urdaneta, 2016-02-26 12:43:44]

Patchset #7 (id:120001) has been deleted

[Guido Urdaneta, 2016-02-26 12:43:51]

Patchset #7 (id:140001) has been deleted