Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(718)

Issue 848533003: Move WeakPtrFactory to the end of AsyncHandleWaiter (Closed)

Created:
5 years, 11 months ago by dmichael (off chromium)
Modified:
5 years, 11 months ago
Reviewers:
Hajime Morrita
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move WeakPtrFactory to the end of AsyncHandleWaiter This forces WeakPtrs to be invalidated before other members' destructors run. See the bug or the documentation of WeakPtrFactory for more information. I'm in the process of trying to turn on a clang check for this (see the bug). BUG=303818 R=morrita@chromium.org Committed: https://crrev.com/e8841f47577c6c1b9eb7dc501762b5f2487bee42 Cr-Commit-Position: refs/heads/master@{#311349}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -5 lines) Patch
M ipc/mojo/async_handle_waiter.h View 1 chunk +1 line, -1 line 0 comments Download
M ipc/mojo/async_handle_waiter.cc View 1 chunk +4 lines, -4 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
dmichael (off chromium)
5 years, 11 months ago (2015-01-13 16:56:52 UTC) #1
Hajime Morrita
lgtm. I did it again so having Clang to check it would be great.
5 years, 11 months ago (2015-01-13 18:31:45 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/848533003/1
5 years, 11 months ago (2015-01-13 18:32:39 UTC) #4
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator/builds/48559)
5 years, 11 months ago (2015-01-13 19:08:04 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/848533003/1
5 years, 11 months ago (2015-01-13 21:25:11 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 11 months ago (2015-01-13 22:32:12 UTC) #9
commit-bot: I haz the power
5 years, 11 months ago (2015-01-13 22:33:05 UTC) #10
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/e8841f47577c6c1b9eb7dc501762b5f2487bee42
Cr-Commit-Position: refs/heads/master@{#311349}

Powered by Google App Engine
This is Rietveld 408576698