[Media Router] Force mDNS discovery on extension wakeup [chromium/src : master]
Bin Zhao (Gerrit)
This change is ready for review.
To view, visit change 630677. To unsubscribe, or for help writing mail filters, visit settings.
Derek Cheng (Gerrit)
High level comment: what is the purpose of hooking up ForceDiscovery to extension wakeups? IMO, OnUserGesture would be a better place for it, as it gets invoked when a MR dialog is being opened.
1 comment:
File components/cast_channel/cast_socket_service.cc:
Patch Set #1, Line 26:
kConnectLivenessTimeoutSecsHmm, I would probably leave this out of this patch. Increased ping timeout across the board without fully understanding its impact might be a bit risky.
To view, visit change 630677. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
+1 for hooking into OnUserGesture - as the extension might wake up for many reasons, some of which might not require forcing discovery. OnUserGesture() is called when the user opens the dialog, and that happens far less often than extension wakeups.
1 comment:
File chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc:
Patch Set #1, Line 126:
dns_sd_registry_->ForceDiscovery();Would this make more sense after L132?
To view, visit change 630677. To unsubscribe, or for help writing mail filters, visit settings.
Derek Cheng (Gerrit)
Patch set 2:Code-Review +1
1 comment:
File chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h:
Patch Set #2, Line 55:
Forcenit: Forces. Also comment that this is a no-op unless the service is started?
To view, visit change 630677. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
To view, visit change 630677. To unsubscribe, or for help writing mail filters, visit settings.
Bin Zhao (Gerrit)
Patch set 3:Commit-Queue +2
Commit Bot (Gerrit)
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"resolve code review comments from Derek" https://chromium-review.googlesource.com/c/630677/3
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/630677/3
Bot data: {"action": "start", "triggered_at": "2017-08-31T18:23:49.0Z", "cq_cfg_revision": "81f1a3f0cb07446abeff7dd01ff1b8bae94ffe0d", "revision": "2c50b1f782ae21c58573519f7714665ffd67c37c"}
Commit Bot (Gerrit)
Commit Bot merged this change.
[Media Router] Force mDNS discovery on extension wakeup
mDNS event is not fired after replugging a Cast device. Try to fix it by forcing mDNS discovery on extension wakeup (which would invoke CastMediaSinkService::Start())
Change liveness timeout to 30s. It is only used for in Browser Cast discovery. Will decrease the value when retry logic is implemented.
Bug: 757891
Change-Id: I6360253406ee289edf679e009d4ec5e4ab177cd1
Reviewed-on: https://chromium-review.googlesource.com/630677
Commit-Queue: Bin Zhao <zha...@chromium.org>
Reviewed-by: Derek Cheng <imc...@chromium.org>
Reviewed-by: mark a. foltz <mfo...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498927}
---
M chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
M chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
M chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
M chrome/browser/media/router/mojo/media_router_mojo_impl.cc
4 files changed, 26 insertions(+), 1 deletion(-)