[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

Extend DisplaySourceConnectionDelegate::Connection interface to support
control message exchange with the connected sink.

Also connection error notification method is added.

BUG=242107
Committed: https://crrev.com/8f13e03e223400dc7e7807470eb16f3c1e9bc4da
Cr-Commit-Position: refs/heads/master@{#370661}

[Mikhail, 2016-01-19 13:16:19]

Description was changed from

==========
[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

Extend DisplaySourceConnectionDelegate::Connection interface to support
control message exchange with the connected sink.

Also connection error notification method is added.

BUG=242107
==========

to

==========
[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

Extend DisplaySourceConnectionDelegate::Connection interface to support
control message exchange with the connected sink.

Also connection error notification method is added.

BUG=242107
==========

[Mikhail, 2016-01-19 13:16:20]

mikhail.pozdnyakov@intel.com changed reviewers:
+ asargent@chromium.org

[Mikhail, 2016-01-19 13:53:33]

Please take a look

[shalamov, 2016-01-19 18:23:41]

alexander.shalamov@intel.com changed reviewers:
+ alexander.shalamov@intel.com

[shalamov, 2016-01-19 18:23:42]

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
File
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.cc
(right):

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.cc:106:
delegate_->Disconnect(on_error);
own_sink_ = kInvalidSinkId; ?

[Mikhail, 2016-01-19 20:02:54]

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
File
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.cc
(right):

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.cc:106:
delegate_->Disconnect(on_error);
On 2016/01/19 18:23:42, shalamov wrote:
> own_sink_ = kInvalidSinkId; ?
that happens at WiFiDisplaySessionServiceImpl::OnSinksUpdated

[shalamov, 2016-01-20 08:02:52]

On 2016/01/19 20:02:54, Mikhail wrote:
>
https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
> File
>
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.cc
> (right):
> 
>
https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
>
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.cc:106:
> delegate_->Disconnect(on_error);
> On 2016/01/19 18:23:42, shalamov wrote:
> > own_sink_ = kInvalidSinkId; ?
> that happens at WiFiDisplaySessionServiceImpl::OnSinksUpdated
lgtm

[asargent_no_longer_on_chrome, 2016-01-20 19:26:17]

lgtm with a couple of nits/suggestions

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
File extensions/browser/api/display_source/display_source_connection_delegate.h
(right):

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/display_source_connection_delegate.h:44:
// If an error occurres 'Observer::OnConnectionError' is invoked.
typo: "occurres" should be "occurs"

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/display_source_connection_delegate.h:48:
// If an error occurres 'Observer::OnConnectionError' is invoked.
same typo: "occurres" should be "occurs"

Also, it might be worth mentioning here whether it is considered valid to call
this method more than once, or if it should only be called once in the lifetime
of each Connection object.

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/display_source_connection_delegate.h:67:
// a transport layer error.
It might be worth mentioning if this event firing implies that the connection
becomes broken/disconnected. (Or can it fire for transient errors which a
connection could recover from?)

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
File
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.h
(right):

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.h:50:
// Called if a message received from the connected sink.
grammar nit: this should be something like one of the following:

"Called if a message is received from the connected sink." 
"Called if a message was received from the connected sink."
"Called when a message is received from the connected sink."

[Mikhail, 2016-01-21 09:38:03]

thanks for your comments

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
File extensions/browser/api/display_source/display_source_connection_delegate.h
(right):

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/display_source_connection_delegate.h:44:
// If an error occurres 'Observer::OnConnectionError' is invoked.
On 2016/01/20 19:26:17, Antony Sargent wrote:
> typo: "occurres" should be "occurs"

Done.

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/display_source_connection_delegate.h:48:
// If an error occurres 'Observer::OnConnectionError' is invoked.
On 2016/01/20 19:26:17, Antony Sargent wrote:
> same typo: "occurres" should be "occurs"
> 
> Also, it might be worth mentioning here whether it is considered valid to call
> this method more than once, or if it should only be called once in the
lifetime
> of each Connection object.
indeed, thanks for the proposal

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/display_source_connection_delegate.h:67:
// a transport layer error.
On 2016/01/20 19:26:17, Antony Sargent wrote:
> It might be worth mentioning if this event firing implies that the connection
> becomes broken/disconnected. (Or can it fire for transient errors which a
> connection could recover from?)
> 

Done.

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
File
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.h
(right):

https://codereview.chromium.org/1603013002/diff/1/extensions/browser/api/disp...
extensions/browser/api/display_source/wifi_display/wifi_display_session_service_impl.h:50:
// Called if a message received from the connected sink.
On 2016/01/20 19:26:17, Antony Sargent wrote:
> grammar nit: this should be something like one of the following:
> 
> "Called if a message is received from the connected sink." 
> "Called if a message was received from the connected sink."
> "Called when a message is received from the connected sink."

Done.

[Mikhail, 2016-01-21 09:38:17]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[Mikhail, 2016-01-21 09:38:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
asargent@chromium.org, alexander.shalamov@intel.com
Link to the patchset: https://codereview.chromium.org/1603013002/#ps20001
(title: "Comments from Antony")

[commit-bot: I haz the power, 2016-01-21 09:38:32]

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

[commit-bot: I haz the power, 2016-01-21 10:27:49]

Description was changed from

==========
[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

Extend DisplaySourceConnectionDelegate::Connection interface to support
control message exchange with the connected sink.

Also connection error notification method is added.

BUG=242107
==========

to

==========
[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

Extend DisplaySourceConnectionDelegate::Connection interface to support
control message exchange with the connected sink.

Also connection error notification method is added.

BUG=242107
==========

[commit-bot: I haz the power, 2016-01-21 10:27:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-21 10:28:44]

Description was changed from

==========
[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

Extend DisplaySourceConnectionDelegate::Connection interface to support
control message exchange with the connected sink.

Also connection error notification method is added.

BUG=242107
==========

to

==========
[chrome.displaySource] DisplaySourceConnectionDelegate::Connection interface

Extend DisplaySourceConnectionDelegate::Connection interface to support
control message exchange with the connected sink.

Also connection error notification method is added.

BUG=242107

Committed: https://crrev.com/8f13e03e223400dc7e7807470eb16f3c1e9bc4da
Cr-Commit-Position: refs/heads/master@{#370661}
==========

[commit-bot: I haz the power, 2016-01-21 10:28:45]

Patchset 2 (id:??) landed as
https://crrev.com/8f13e03e223400dc7e7807470eb16f3c1e9bc4da
Cr-Commit-Position: refs/heads/master@{#370661}