New issue
Advanced search Search tips

Issue 887758 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 21
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

DeviceReenroller should remove itself as an observer in destructor

Project Member Reported by nohle@chromium.org, Sep 20

Issue description

Currently DeviceReenroller neglects to remove itself as a DeviceSyncClient::Observer in the destructor.
 
Labels: -Pri-2 Pri-3
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/074bead6f49634abfa42f9ac73e444502d3e924d

commit 074bead6f49634abfa42f9ac73e444502d3e924d
Author: Josh Nohle <nohle@google.com>
Date: Fri Sep 21 01:09:46 2018

[CrOS MultiDevice] DeviceReenroller: Remove self as observer in dtor

Because DeviceReenroller is a DeviceSyncClient::Observer, it should
remove itself as an observer in the destructor.

Bug:  887557 ,  887758 
Change-Id: I318ee3a3a9f3b7df6cef1bfb362049a6687b9436
Reviewed-on: https://chromium-review.googlesource.com/1237536
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593035}
[modify] https://crrev.com/074bead6f49634abfa42f9ac73e444502d3e924d/chromeos/services/multidevice_setup/device_reenroller.cc

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/135c87489dd68bba667c8b4464084aa089df84c7

commit 135c87489dd68bba667c8b4464084aa089df84c7
Author: Josh Nohle <nohle@google.com>
Date: Fri Sep 21 16:37:57 2018

[CrOS MultiDevice] DeviceReenroller: Reorder unit test member variables to fix ASAN build

Because DeviceReenroller now uses the DeviceSyncClient in the destructor
to remove itself as a DeviceSyncClient::Observer, DeviceReenroller needs
to be destroyed before the DeviceSyncClient in the unit tests.

Bug:  887953 ,  887557 ,  887758 
Change-Id: I23e8f150ef08e6ad3d388f4ed86d47123077f71a
Tested: Unit tests with ASAN on
Reviewed-on: https://chromium-review.googlesource.com/1238818
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593220}
[modify] https://crrev.com/135c87489dd68bba667c8b4464084aa089df84c7/chromeos/services/multidevice_setup/device_reenroller_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 24

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d3ddae3f6656ad3134b645c9171e96859355d46

commit 9d3ddae3f6656ad3134b645c9171e96859355d46
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Sep 24 21:33:13 2018

[CrOS MultiDevice] DeviceReenroller: Remove self as observer in dtor

Because DeviceReenroller is a DeviceSyncClient::Observer, it should
remove itself as an observer in the destructor.

TBR=nohle@google.com

(cherry picked from commit 074bead6f49634abfa42f9ac73e444502d3e924d)

Bug:  887557 ,  887758 
Change-Id: I318ee3a3a9f3b7df6cef1bfb362049a6687b9436
Reviewed-on: https://chromium-review.googlesource.com/1237536
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593035}
Reviewed-on: https://chromium-review.googlesource.com/1241566
Cr-Commit-Position: refs/branch-heads/3538@{#607}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/9d3ddae3f6656ad3134b645c9171e96859355d46/chromeos/services/multidevice_setup/device_reenroller.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd2cdd6d64c45b598e438def6c561faca7bd1f10

commit dd2cdd6d64c45b598e438def6c561faca7bd1f10
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Sep 24 21:34:45 2018

[CrOS MultiDevice] DeviceReenroller: Reorder unit test member variables to fix ASAN build

Because DeviceReenroller now uses the DeviceSyncClient in the destructor
to remove itself as a DeviceSyncClient::Observer, DeviceReenroller needs
to be destroyed before the DeviceSyncClient in the unit tests.

TBR=nohle@google.com

(cherry picked from commit 135c87489dd68bba667c8b4464084aa089df84c7)

Bug:  887953 ,  887557 ,  887758 
Change-Id: I23e8f150ef08e6ad3d388f4ed86d47123077f71a
Tested: Unit tests with ASAN on
Reviewed-on: https://chromium-review.googlesource.com/1238818
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593220}
Reviewed-on: https://chromium-review.googlesource.com/1241753
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#609}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/dd2cdd6d64c45b598e438def6c561faca7bd1f10/chromeos/services/multidevice_setup/device_reenroller_unittest.cc

Sign in to add a comment