Presentation API: implement .getAvailability() (Chromium side).

Presentation API: implement .getAvailability() (Chromium side).

This is part of a suite of three CLs:
1. https://codereview.chromium.org/1222623003
2. <this>
3. https://codereview.chromium.org/1220813009

BUG=498435
Committed: https://crrev.com/8106b377c40257b589e517c3c91c3bc2ae3086b0
Cr-Commit-Position: refs/heads/master@{#337502}

[mlamouri (slow - plz ping), 2015-07-02 16:40:02]

mlamouri@chromium.org changed reviewers:
+ avayvod@chromium.org

[whywhat, 2015-07-02 22:42:48]

lgtm with nits and code improvement suggestions

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:343:
previous_availability_ = available;
nit: check if the last known availability is different and only then fire
availabilityChanged() ?

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:456: 
maybe something like this would be shorter and easier to follow?

bool is_listening = listening_state_ != ListeningState::Inactive;
if (should_listen == is_listening)
  return;

ConnectToPresentationServiceIfNeeded();
if (should_listen) {
  listening_state_ = ListeningState::Waiting;
  presentation_service_->ListenForScreenAvailability();
} else {
  listening_state_ = ListeningState::Inactive;
  presentation_service_->StopListeningForScreenAvailability();
}

If you only had two states (or a boolean) it would probably be even shorter.

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
File content/renderer/presentation/presentation_dispatcher.h (right):

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.h:109: Waiting,
This state is not used, only to assign it to the listening_, but never checked.

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.h:113: ListeningState
listening_;
s/listening_/listening_state_

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.h:114: bool
previous_availability_;
nit: previous to what? last_known_availability_ maybe?

[mlamouri (slow - plz ping), 2015-07-03 13:35:46]

Thanks for the review! Comments applied.

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:343:
previous_availability_ = available;
On 2015/07/02 at 22:42:48, whywhat wrote:
> nit: check if the last known availability is different and only then fire
availabilityChanged() ?

This is being checked before sending the event. Probably not useful to do the
check twice.

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:456: 
On 2015/07/02 at 22:42:48, whywhat wrote:
> maybe something like this would be shorter and easier to follow?
> 
> bool is_listening = listening_state_ != ListeningState::Inactive;
> if (should_listen == is_listening)
>   return;
> 
> ConnectToPresentationServiceIfNeeded();
> if (should_listen) {
>   listening_state_ = ListeningState::Waiting;
>   presentation_service_->ListenForScreenAvailability();
> } else {
>   listening_state_ = ListeningState::Inactive;
>   presentation_service_->StopListeningForScreenAvailability();
> }
> 
> If you only had two states (or a boolean) it would probably be even shorter.

Done.

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
File content/renderer/presentation/presentation_dispatcher.h (right):

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.h:109: Waiting,
On 2015/07/02 at 22:42:48, whywhat wrote:
> This state is not used, only to assign it to the listening_, but never
checked.

It is not directly used but indirectly. When getAvailability() is called, if the
listening is WAITING, we will add the callback. If it is ACTIVE, we will resolve
it directly. It's different from INACTIVE because in that case, we should start
listening.

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.h:113: ListeningState
listening_;
On 2015/07/02 at 22:42:48, whywhat wrote:
> s/listening_/listening_state_

Done.

https://codereview.chromium.org/1214083013/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.h:114: bool
previous_availability_;
On 2015/07/02 at 22:42:48, whywhat wrote:
> nit: previous to what? last_known_availability_ maybe?

Done.

[mlamouri (slow - plz ping), 2015-07-06 21:55:53]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-07-06 21:55:53]

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

[commit-bot: I haz the power, 2015-07-06 21:56:14]

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

[commit-bot: I haz the power, 2015-07-06 22:44:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-07-06 22:46:02]

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