Skip to content

Commit

Permalink
Remove the MojoChannelAssociatedCrashesOnSendError feature.
Browse files Browse the repository at this point in the history
Stable experiment demonstrates that SendMessageOnSequence can only fail
if `connector_->encountered_error()` is true, so we make the CHECK no
longer gated by a feature.

Bug: 40944462
Change-Id: Idcbf7e3d209468db1cc8e90d901a6f37f3b65abf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5954810
Commit-Queue: Ken Rockot <[email protected]>
Auto-Submit: Francois Pierre Doray <[email protected]>
Reviewed-by: Ken Rockot <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1375332}
  • Loading branch information
fdoray authored and Chromium LUCI CQ committed Oct 29, 2024
1 parent 6bffb0a commit fb40835
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 41 deletions.
25 changes: 6 additions & 19 deletions ipc/ipc_mojo_bootstrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "base/memory/ptr_util.h"
#include "base/memory/raw_ptr.h"
#include "base/no_destructor.h"
#include "base/not_fatal_until.h"
#include "base/ranges/algorithm.h"
#include "base/sequence_checker.h"
#include "base/strings/stringprintf.h"
Expand Down Expand Up @@ -68,10 +69,6 @@ BASE_FEATURE(kMojoChannelAssociatedSendUsesRunOrPostTask,
"MojoChannelAssociatedSendUsesRunOrPostTask",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kMojoChannelAssociatedCrashesOnSendError,
"MojoChannelAssociatedCrashesOnSendError",
base::FEATURE_DISABLED_BY_DEFAULT);

// Used to track some internal Channel state in pursuit of message leaks.
//
// TODO(crbug.com/40563310): Remove this.
Expand Down Expand Up @@ -918,24 +915,14 @@ class ChannelAssociatedGroupController
}
return true;
}
MojoResult result = connector_->AcceptAndGetResult(message);

// TODO(crbug.com/40944462): Remove this code when the cause of skipped
// messages with MojoChannelAssociatedSendUsesRunOrPostTask is understood,
// or no later than November 2024.
if (result != MOJO_RESULT_OK && !connector_->encountered_error() &&
base::FeatureList::IsEnabled(
kMojoChannelAssociatedCrashesOnSendError)) {
// Crash when sending a message fails and `connector_` can send more
// messages, as that breaks the assumption that messages are received in
// the order they were sent. Note: `connector_` cannot send more messages
// when `encountered_error()` is true.
mojo::debug::ScopedMessageErrorCrashKey crash_key(
base::StringPrintf("SendMessage failed with error %d", result));
CHECK(false);
MojoResult result = connector_->AcceptAndGetResult(message);
if (result == MOJO_RESULT_OK) {
return true;
}

return result == MOJO_RESULT_OK;
CHECK(connector_->encountered_error(), base::NotFatalUntil::M135);
return false;
}

void SendMessageOnSequenceViaTask(mojo::Message message) {
Expand Down
22 changes: 0 additions & 22 deletions testing/variations/fieldtrial_testing_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -14165,28 +14165,6 @@
]
}
],
"MojoChannelAssociatedCrashesOnSendError": [
{
"platforms": [
"android",
"android_webview",
"chromeos",
"chromeos_lacros",
"fuchsia",
"linux",
"mac",
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"MojoChannelAssociatedCrashesOnSendError"
]
}
]
}
],
"MojoChannelAssociatedSendUsesRunOrPostTask": [
{
"platforms": [
Expand Down

0 comments on commit fb40835

Please sign in to comment.