[Media Router] Connection reattempt logic and bound pending request queue size.

[Media Router] Connection reattempt logic and bound pending request queue size.

This patch address the following corner cases:

1. If OnConnectionError() is invoked while there are queued requests, this
happens when extension is woken up but MRPM did not register with MR
successfully.
2. Wake up extension failed.
3. After connection is established (RegisterMediaRouteProvider), we try to run
all pending requests, but it's possible that the extension is already suspended
at that point. It looks like an uncommon timing issue and does not appear to
indicate a connection error.

For 1 and 3, we will retry the wakeup (up to a max number of attempts) since
there is no way to tell if the error was transient. For 2, it is usually due to
a nonretriable error, so we will just drain the queue.

The retry / drain logic also ensures pending requests do not sit in the queue
indefinitely.

Also, introduced a max limit queue size. When the queue size exceeds that limit,
the oldest pending request will be dropped.

A couple of things considered but not implemented:
- Try to fail the pending request in some way instead of dropping them. First,
not all types of requests can be failed, i.e. majority of them do not contain
callbacks. Also because it is a corner case, the added complexity probably does
not justify it.
- Adding a permanent_failure bit to MediaRouterMojoImpl and automatically drop
requests if set to true. Locking down the media router due to connection error
feels too restrictive and I am not sure if it will help.

BUG=490787
Committed: https://crrev.com/3dbd73f561741c78541d1a323706f337fc4717d5
Cr-Commit-Position: refs/heads/master@{#357047}

[imcheng, 2015-10-21 20:48:27]

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

[imcheng, 2015-10-27 23:18:41]

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

[imcheng, 2015-10-27 23:18:42]

+Kevin since mark is OOO

[Kevin M, 2015-10-27 23:25:40]

https://codereview.chromium.org/1419853003/diff/20001/chrome/browser/media/ro...
File chrome/browser/media/router/media_router_mojo_impl_unittest.cc (right):

https://codereview.chromium.org/1419853003/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_mojo_impl_unittest.cc:789:
EXPECT_CALL(*process_manager_, WakeEventPage(kExtensionId, _))
Set this outside the loop with .Times(kMaxWakeupAttemptCount-1)

https://codereview.chromium.org/1419853003/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_mojo_impl_unittest.cc:797:
EXPECT_TRUE(media_router_->pending_requests_.empty());
Test that the queue builds up back again following a drain?

[imcheng, 2015-10-27 23:32:01]

https://codereview.chromium.org/1419853003/diff/20001/chrome/browser/media/ro...
File chrome/browser/media/router/media_router_mojo_impl_unittest.cc (right):

https://codereview.chromium.org/1419853003/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_mojo_impl_unittest.cc:789:
EXPECT_CALL(*process_manager_, WakeEventPage(kExtensionId, _))
On 2015/10/27 23:25:40, Kevin M wrote:
> Set this outside the loop with .Times(kMaxWakeupAttemptCount-1)

Done.

https://codereview.chromium.org/1419853003/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_mojo_impl_unittest.cc:797:
EXPECT_TRUE(media_router_->pending_requests_.empty());
On 2015/10/27 23:25:40, Kevin M wrote:
> Test that the queue builds up back again following a drain?

Done.

[imcheng, 2015-10-28 00:32:46]

The tests were fixed. Also fixed a teardown issue (MRMojoImpl has to be
destroyed before the profile)

[Kevin M, 2015-10-30 00:37:42]

lgtm

https://codereview.chromium.org/1419853003/diff/100001/chrome/browser/media/r...
File chrome/browser/media/router/media_router_mojo_impl.cc (right):

https://codereview.chromium.org/1419853003/diff/100001/chrome/browser/media/r...
chrome/browser/media/router/media_router_mojo_impl.cc:732: if
(wakeup_attempt_count_ >= kMaxWakeupAttemptCount) {
GE comparison for this type of thing is unusual.

Increment wakeup_attempt_count before the "if" and just do greater-than?

[imcheng, 2015-10-30 04:39:14]

The CQ bit was checked by imcheng@chromium.org

[imcheng, 2015-10-30 04:39:14]

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

[commit-bot: I haz the power, 2015-10-30 04:39:25]

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

[imcheng, 2015-10-30 04:39:35]

https://codereview.chromium.org/1419853003/diff/100001/chrome/browser/media/r...
File chrome/browser/media/router/media_router_mojo_impl.cc (right):

https://codereview.chromium.org/1419853003/diff/100001/chrome/browser/media/r...
chrome/browser/media/router/media_router_mojo_impl.cc:732: if
(wakeup_attempt_count_ >= kMaxWakeupAttemptCount) {
On 2015/10/30 00:37:42, Kevin M wrote:
> GE comparison for this type of thing is unusual.
> 
> Increment wakeup_attempt_count before the "if" and just do greater-than?

Done.

[commit-bot: I haz the power, 2015-10-30 04:43:24]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-10-30 04:44:09]

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