chromium/src
0
Fork 0
Code Activity

[fuchsia] Close async completer on destruction of server.

Browse Source 
FIDL completers on two-way methods trigger a CHECK error if
`Reply` or `Close` isn't called before they are destroyed.
This CL adds a call to `Close` for the `WatchInfoChangeCompleter`
that is being held by `MediaPlayerImpl` to cover that case.

Bug: 1429266
Change-Id: I35dd8f3b9565614b64cc4765ddb0dfb91f7b12ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4384573
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Commit-Queue: Bryant Chandler <bryantchandler@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1124255}
This commit is contained in:
Bryant Chandler
2023-03-30 16:45:16 +00:00
committed by Chromium LUCI CQ
parent cb11205f7e
commit 2515e12a80
2 changed files with 22 additions and 3 deletions

6
fuchsia_web/webengine/browser/media_player_impl.cc

@@ -103,7 +103,11 @@ MediaPlayerImpl::MediaPlayerImpl(
pending_info_delta_.local(true);
}
MediaPlayerImpl::~MediaPlayerImpl() = default;
MediaPlayerImpl::~MediaPlayerImpl() {
if (pending_info_change_callback_) {
pending_info_change_callback_->Close(ZX_ERR_PEER_CLOSED);
}
}
void MediaPlayerImpl::WatchInfoChange(
MediaPlayerImpl::WatchInfoChangeCompleter::Sync& completer) {

19
fuchsia_web/webengine/browser/media_player_impl_unittest.cc

@@ -104,7 +104,7 @@ class MediaPlayerImplTest : public testing::Test {
std::unique_ptr<MediaPlayerImpl> player_impl_;
};
// Verify that the |on_disconnect| closure is invoked if the client disconnects.
// Verify that the `on_disconnect` closure is invoked if the client disconnects.
TEST_F(MediaPlayerImplTest, OnDisconnectCalledOnDisconnect) {
base::RunLoop run_loop;
player_impl_ = std::make_unique<MediaPlayerImpl>(
@@ -113,7 +113,7 @@ TEST_F(MediaPlayerImplTest, OnDisconnectCalledOnDisconnect) {
run_loop.Run();
}
// Verify that the |on_disconnect| closure is invoked if the client calls the
// Verify that the `on_disconnect` closure is invoked if the client calls the
// WatchInfoChange() API incorrectly.
TEST_F(MediaPlayerImplTest, ClientDisconnectedOnBadApiUsage) {
base::RunLoop on_disconnected_loop;
@@ -145,6 +145,21 @@ TEST_F(MediaPlayerImplTest, ClientDisconnectedOnBadApiUsage) {
player_error_loop.Run();
}
// Verify that the completer is Closed on destruction of `MediaPlayerImpl`.
// Otherwise it will trigger a CHECK for failing to reply.
TEST_F(MediaPlayerImplTest, WatchInfoChangeAsyncCompleterClosedOnDestruction) {
player_impl_ = std::make_unique<MediaPlayerImpl>(
&fake_session_, std::move(player_server_end_),
MakeExpectedNotRunClosure(FROM_HERE));
player_->WatchInfoChange().Then([](auto result) {});
// The first call always replies immediately, so call a second call to hold a
// pending completer.
player_->WatchInfoChange().Then([](auto result) {});
// Pump the message loop to process the WatchInfoChange() call.
base::RunLoop().RunUntilIdle();
}
// Verify that the first WatchInfoChange() registers the observer.
TEST_F(MediaPlayerImplTest, WatchInfoChangeRegistersObserver) {
player_impl_ = std::make_unique<MediaPlayerImpl>(