Commit 6c82fd92 authored by Xiyuan Xia's avatar Xiyuan Xia Committed by Commit Bot

Fix msan problem in MultiProfileFileManagerBrowserTest

The cause is a typical mojo message race. SessionController and
MediaController use different mojo message pipes. The problem
happens when two or more users are added to the system and somehow
MediaController call is delayed after SessionController calls.
As a result, MediaController calls contains a stale capture states
array (less than actual users). MultiProfileMediaTrayView uses
a user index based on SessionController's view and would hit an
index out of bound error when accessing the stale data.

Make MultiProfileMediaTrayView::OnMediaCaptureChanged use size of
the passed-in |capture_states| to fix the problem. This should be
fine since it is transient and there should soon be another
MediaController::NotifyCaptureState call.

Bug: 842442
Change-Id: Ife6ea23c85d27e2e89a042bb849ef03af4ccc061
Reviewed-on: https://chromium-review.googlesource.com/1062716Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559323}
parent 8c534d48
......@@ -7,7 +7,6 @@
#include "ash/ash_view_ids.h"
#include "ash/media_controller.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller.h"
#include "ash/shell.h"
#include "ash/system/tray/system_tray_notifier.h"
#include "ash/system/tray/tray_constants.h"
......@@ -39,10 +38,8 @@ class MultiProfileMediaTrayView : public TrayItemView,
// MediaCaptureObserver:
void OnMediaCaptureChanged(
const std::vector<mojom::MediaCaptureState>& capture_states) override {
SessionController* controller = Shell::Get()->session_controller();
// The user at 0 is the current desktop user.
for (UserIndex index = 1; index < controller->NumberOfLoggedInUsers();
++index) {
// The user at 0 is the current active desktop user.
for (size_t index = 1; index < capture_states.size(); ++index) {
if (capture_states[index] != mojom::MediaCaptureState::NONE) {
SetVisible(true);
return;
......
......@@ -473,45 +473,22 @@ class MultiProfileFileManagerBrowserTest : public FileManagerBrowserTestBase {
DISALLOW_COPY_AND_ASSIGN(MultiProfileFileManagerBrowserTest);
};
// Flaky crashes in ash::tray::MultiProfileMediaTrayView.
// https://crbug.com/842442
#if defined(MEMORY_SANITIZER) || defined(ADDRESS_SANITIZER) || \
defined(LEAK_SANITIZER)
#define MAYBE_PRE_BasicDownloads DISABLED_PRE_BasicDownloads
#define MAYBE_BasicDownloads DISABLED_BasicDownloads
#else
#define MAYBE_PRE_BasicDownloads PRE_BasicDownloads
#define MAYBE_BasicDownloads BasicDownloads
#endif
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest,
MAYBE_PRE_BasicDownloads) {
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest, PRE_BasicDownloads) {
AddAllUsers();
}
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest,
MAYBE_BasicDownloads) {
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest, BasicDownloads) {
AddAllUsers();
// Sanity check that normal operations work in multi-profile.
set_test_case_name("keyboardCopyDownloads");
StartTest();
}
// Flaky crashes in ash::tray::MultiProfileMediaTrayView.
// https://crbug.com/842442
#if defined(MEMORY_SANITIZER) || defined(ADDRESS_SANITIZER) || \
defined(LEAK_SANITIZER)
#define MAYBE_PRE_BasicDrive DISABLED_PRE_BasicDrive
#define MAYBE_BasicDrive DISABLED_BasicDrive
#else
#define MAYBE_PRE_BasicDrive PRE_BasicDrive
#define MAYBE_BasicDrive BasicDrive
#endif
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest,
MAYBE_PRE_BasicDrive) {
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest, PRE_BasicDrive) {
AddAllUsers();
}
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest, MAYBE_BasicDrive) {
IN_PROC_BROWSER_TEST_F(MultiProfileFileManagerBrowserTest, BasicDrive) {
AddAllUsers();
// Sanity check that normal operations work in multi-profile.
set_test_case_name("keyboardCopyDrive");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment