Jump to conversation
Unresolved conversations (6)
@iefremov iefremov Jan 18, 2021
(sorry, just randomly came across this) we really shouldn't use `base::Unretained` like this, because `this` can easily die before the task is started (especially given that most tasks posted around can be time consuming). Note, that `CreateTaskCompletionClosure` binds a weak pointer. Fortunately, in our case `WaitForIPFSRepoGC` doesn't use that pointer at all, so we shouldn't crash. The most easy fix would be to just post a free function (i.e. not a member of `BraveBrowsingDataRemoverDelegate`) - there are examples of `WaitForExitWithTimeout` across the codebase
...a/brave_browsing_data_remover_delegate.cc
@yrliou @iefremov
@mkarolin mkarolin Jan 15, 2021
Nit: empty line after a multi-line `#define`
..._src/base/threading/thread_restrictions.h
@yrliou
@mkarolin mkarolin Jan 15, 2021
Not sure I am seeing why we need the `friend` here.
...a/chrome_browsing_data_remover_delegate.h
@yrliou
@bbondy bbondy Jan 15, 2021
We should get the last known good path when the executable path is not determined here please in a follow up.
...a/brave_browsing_data_remover_delegate.cc
@yrliou
@yrliou yrliou Jan 14, 2021
This does not have the dependency added in this PR. We have some existing deps problems for the browsing_data target, which I would like it to be addressed in a separate PR. For chromium, their browsing_data_remover_delegate source files are listed in the browser target itself, I think we should probably consider doing the same since we need chrome/browser and brave/browser in existing files in our browsing_data target.
...a/brave_browsing_data_remover_delegate.cc
@yrliou yrliou Jan 14, 2021
kHostCache is the most reasonable existing one I could find in the list, type value is only meant for recording purpose, so we should be good here. This value is gone in chromium latest version, so we would need to update the value to kEmbedderData during future chromium upgrade.
...a/brave_browsing_data_remover_delegate.cc
Resolved conversations (0)