Presentation API: implement renderer side of PresentationConnection.close().

Presentation API: implement renderer side of PresentationConnection.close().

This is basically implementing the Blink side and the plumbing going to
the Media Router.

Intent to ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/24gzqKMiiR8

Spec:
https://w3c.github.io/presentation-api/

BUG=540803
Committed: https://crrev.com/c65a70d71da5f3643889d4995a7cc13153c08cf4
Cr-Commit-Position: refs/heads/master@{#362009}

[mlamouri (slow - plz ping), 2015-11-20 01:17:00]

mlamouri@chromium.org changed reviewers:
+ avayvod@chromium.org, isherman@chromium.org, jochen@chromium.org,
mfoltz@chromium.org

[mlamouri (slow - plz ping), 2015-11-20 01:17:00]

Could you review:

jochen@
 third_party/WebKit/Source/core/frame/UseCounter.h
 virtual/stable/webexposed/

isherman@:
 tools/metrics/histograms/histograms.xml

avayvod@, mfoltz@:
 everything else

Thanks :)

[whywhat, 2015-11-20 06:26:49]

lgtm with nits and suggestions

https://codereview.chromium.org/1466573002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/presentation_service_delegate_impl.cc (right):

https://codereview.chromium.org/1466573002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/presentation_service_delegate_impl.cc:735:
NOTIMPLEMENTED();
This can probably implemented similar to onPresentationConnectionDetached()? The
MR is notified that the MediaRoute is no longer attached to the page.

https://codereview.chromium.org/1466573002/diff/1/content/public/browser/pres...
File content/public/browser/presentation_service_delegate.h (right):

https://codereview.chromium.org/1466573002/diff/1/content/public/browser/pres...
content/public/browser/presentation_service_delegate.h:155: // Listen for
messages for a presentation session.
nit: should you add 's' to the leading verb here and for the
ListenForStateChange below?

https://codereview.chromium.org/1466573002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp
(right):

https://codereview.chromium.org/1466573002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp:319:
if (m_state != WebPresentationConnectionState::Connected)
who sets the state of the session to Closed or Terminated? is it done by the
browser side and then propagated back to PresentationConnection?

[imcheng, 2015-11-20 06:31:45]

imcheng@chromium.org changed reviewers:
+ imcheng@chromium.org

[imcheng, 2015-11-20 06:31:45]

https://codereview.chromium.org/1466573002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp
(right):

https://codereview.chromium.org/1466573002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp:319:
if (m_state != WebPresentationConnectionState::Connected)
On 2015/11/20 06:26:49, whywhat wrote:
> who sets the state of the session to Closed or Terminated? is it done by the
> browser side and then propagated back to PresentationConnection?

The MRP should notify MR of state change as it handles the
OnPresentationConnectionDetached() or CloseRoute() call. It is then propagated
back to PresentationConnection. See
https://codereview.chromium.org/1430413003/diff/200001/chrome/browser/media/r...

[jochen (gone - plz use gerrit), 2015-11-20 13:37:41]

lgtm

[mark a. foltz, 2015-11-20 17:47:17]

https://codereview.chromium.org/1466573002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/presentation_service_delegate_impl.cc (right):

https://codereview.chromium.org/1466573002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/presentation_service_delegate_impl.cc:735:
NOTIMPLEMENTED();
On 2015/11/20 at 06:26:49, whywhat wrote:
> This can probably implemented similar to onPresentationConnectionDetached()?
The MR is notified that the MediaRoute is no longer attached to the page.

This should call router_->OnPresentationSessionDetached(route_id) to notify the
MRP that the connection is going away.  PresentationFrame::Reset() should
probably call CloseSession(route_id) as part of the frame-level cleanup to
create one code path for this.

Note that Derek just landed a patch last night that changes the APIs to this
file so you will want to rebase.

https://chromium.googlesource.com/chromium/src.git/+/f3e5a01a75bbdd97e5d218c4...

[Ilya Sherman, 2015-11-20 19:29:12]

histograms.xml lgtm

[mlamouri (slow - plz ping), 2015-11-27 11:48:31]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-11-27 11:48:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, avayvod@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1466573002/#ps40001
(title: "review comments")

[mlamouri (slow - plz ping), 2015-11-27 11:48:47]

https://codereview.chromium.org/1466573002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/presentation_service_delegate_impl.cc (right):

https://codereview.chromium.org/1466573002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/presentation_service_delegate_impl.cc:735:
NOTIMPLEMENTED();
On 2015/11/20 at 17:47:17, mfoltz_OOO_until_11_30 wrote:
> On 2015/11/20 at 06:26:49, whywhat wrote:
> > This can probably implemented similar to onPresentationConnectionDetached()?
The MR is notified that the MediaRoute is no longer attached to the page.
> 
> This should call router_->OnPresentationSessionDetached(route_id) to notify
the MRP that the connection is going away.  PresentationFrame::Reset() should
probably call CloseSession(route_id) as part of the frame-level cleanup to
create one code path for this.
> 
> Note that Derek just landed a patch last night that changes the APIs to this
file so you will want to rebase.
> 
>
https://chromium.googlesource.com/chromium/src.git/+/f3e5a01a75bbdd97e5d218c4...

Rebased. I left the TODO. I will have a look in a follow-up but the intent of
this CL was to implement the renderer side, not implement the full feature.

https://codereview.chromium.org/1466573002/diff/1/content/public/browser/pres...
File content/public/browser/presentation_service_delegate.h (right):

https://codereview.chromium.org/1466573002/diff/1/content/public/browser/pres...
content/public/browser/presentation_service_delegate.h:155: // Listen for
messages for a presentation session.
On 2015/11/20 at 06:26:49, whywhat wrote:
> nit: should you add 's' to the leading verb here and for the
ListenForStateChange below?

Done.

https://codereview.chromium.org/1466573002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp
(right):

https://codereview.chromium.org/1466573002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp:319:
if (m_state != WebPresentationConnectionState::Connected)
On 2015/11/20 at 06:31:45, imcheng1 wrote:
> On 2015/11/20 06:26:49, whywhat wrote:
> > who sets the state of the session to Closed or Terminated? is it done by the
> > browser side and then propagated back to PresentationConnection?
> 
> The MRP should notify MR of state change as it handles the
OnPresentationConnectionDetached() or CloseRoute() call. It is then propagated
back to PresentationConnection. See
https://codereview.chromium.org/1430413003/diff/200001/chrome/browser/media/r...

Ack.

[commit-bot: I haz the power, 2015-11-27 11:48:52]

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

[commit-bot: I haz the power, 2015-11-27 14:37:14]

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

[commit-bot: I haz the power, 2015-11-27 14:37:16]

Failed to apply patch for third_party/WebKit/Source/core/frame/UseCounter.h:
While running git apply --index -3 -p1;
  error: patch failed: third_party/WebKit/Source/core/frame/UseCounter.h:891
  Falling back to three-way merge...
  Applied patch to 'third_party/WebKit/Source/core/frame/UseCounter.h' with
conflicts.
  U third_party/WebKit/Source/core/frame/UseCounter.h

Patch:       third_party/WebKit/Source/core/frame/UseCounter.h
Index: third_party/WebKit/Source/core/frame/UseCounter.h
diff --git a/third_party/WebKit/Source/core/frame/UseCounter.h
b/third_party/WebKit/Source/core/frame/UseCounter.h
index
f98c1f1432347cbad3ac3970836fd687ad099825..d3db443d71545a5b6b1ec15140c8741abf8cda01
100644
--- a/third_party/WebKit/Source/core/frame/UseCounter.h
+++ b/third_party/WebKit/Source/core/frame/UseCounter.h
@@ -891,6 +891,7 @@ public:
         OfflineAudioContextResume = 1033,
         AttrCloneNode = 1034,
         SVG1DOMPaintServer = 1035,
+        PresentationConnectionClose = 1036,
 
         // Add new features immediately above this line. Don't change assigned
         // numbers of any item, and don't reuse removed slots.

[commit-bot: I haz the power, 2015-11-27 14:37:29]

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

[mlamouri (slow - plz ping), 2015-11-27 15:05:43]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-11-27 15:05:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, avayvod@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1466573002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2015-11-27 15:06:07]

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

[commit-bot: I haz the power, 2015-11-27 16:29:07]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-27 16:30:03]

Description was changed from

==========
Presentation API: implement renderer side of PresentationConnection.close().

This is basically implementing the Blink side and the plumbing going to
the Media Router.

Intent to ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/24gzqKMiiR8

Spec:
https://w3c.github.io/presentation-api/

BUG=540803
==========

to

==========
Presentation API: implement renderer side of PresentationConnection.close().

This is basically implementing the Blink side and the plumbing going to
the Media Router.

Intent to ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/24gzqKMiiR8

Spec:
https://w3c.github.io/presentation-api/

BUG=540803

Committed: https://crrev.com/c65a70d71da5f3643889d4995a7cc13153c08cf4
Cr-Commit-Position: refs/heads/master@{#362009}
==========

[commit-bot: I haz the power, 2015-11-27 16:30:04]

Patchset 4 (id:??) landed as
https://crrev.com/c65a70d71da5f3643889d4995a7cc13153c08cf4
Cr-Commit-Position: refs/heads/master@{#362009}