Add a web API clearCrashpadDatabase #4790
Conversation
| std::vector<char> cache_dir(kSbFileMaxPath + 1, 0); | ||
| SbSystemGetPath(kSbSystemPathCacheDirectory, cache_dir.data(), | ||
| kSbFileMaxPath); | ||
| base::FilePath crashpad_db_dir = | ||
| base::FilePath(cache_dir.data()).Append(kCrashpadDBName); |
There was a problem hiding this comment.
Could we use std::string?
std::string cache_folder(kSbFileMaxPath, 0);
SbSystemGetPath(..., cache_folder.data(),..);
base::FilePath crashpad_db_folder(cache_folder);
crashpad_db_folder.Append(kCrashpadDBName);
Try to avoid using acronyms or shortenings: s/dir/directory/ or s/dir/folder/. Maybe also expand s/db/database/.
There was a problem hiding this comment.
Thanks for the suggestion. Same as above. I followed the existing style for consistency, including the name "cache_dir". If all reviewers think they should be fixed all together, I'll make another PR.
| std::vector<char> cache_dir(kSbFileMaxPath + 1, 0); | ||
| SbSystemGetPath(kSbSystemPathCacheDirectory, cache_dir.data(), | ||
| kSbFileMaxPath); | ||
| base::FilePath crashpad_db_dir = |
There was a problem hiding this comment.
Declaring and initializing crashpad_db_dir in one line: I don't see a strong reason to have it the way it's presented vs the suggested alternative below unless it's to make this variable const.
There was a problem hiding this comment.
Thanks for the comment. There're 4 existing references of initializing file path and appending another segment in h5vcc_storage.cc. I followed the existing style for consistency. I don't see which way is significantly better or more correct. I prefer to make this PR focused on adding the API, rather than fixing existing styles. I don't think it's absolutely necessary to fix the styles of this file at the moment, as this style is present in many files in the code base. What's the point of fixing one file without fixing all the files? IMO it's not a priority for the team at the moment.
I'll submit this PR as is since all reviewers approved. But if all the reviewers think it's absolutely necessary to fix this style in this file, I'll make another PR to fix this one and the other 4 references, just to make this PR focused. @y4vor @hlwarriner
There was a problem hiding this comment.
I'm fine with keeping the existing style. Thanks for checking.
| @@ -31,4 +31,6 @@ interface H5vccStorage { | |||
| void clearCache(); | |||
| void clearCacheOfType(octet type_index); | |||
| void clearServiceWorkerCache(); | |||
|
|
|||
| void clearCrashpadDatabase(); | |||
There was a problem hiding this comment.
It may be a good idea to just add a quick comment explaining that this API is only meant to be called from tests.
This API will be used in crashpad handler test to clear up the previous states to work around the upload throttling that causes the tests to fail frequently for partner certification. b/332895679 Change-Id: I0b104a64dbd156de6df817800c972042e810e654
This API will be used in crashpad handler test to clear up the previous states to work around the upload throttling that causes the tests to fail frequently for partner certification.
Verified on Linux: Crashpad DB is purged after caling the API. After app restarts, crash upload works as expected.
b/332895679
Change-Id: I0b104a64dbd156de6df817800c972042e810e654