inspector: auto collect webstorage data#62145
Conversation
|
Review requested:
|
nodejs-github-bot
added
c++
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62145 +/- ##
==========================================
- Coverage 89.64% 89.64% -0.01%
==========================================
Files 676 677 +1
Lines 206240 206713 +473
Branches 39514 39576 +62
==========================================
+ Hits 184891 185306 +415
- Misses 13465 13500 +35
- Partials 7884 7907 +23
🚀 New features to boost your workflow:
|
| ? std::make_unique<std::unordered_map<std::string, std::string>>( | ||
| local_storage_map_) | ||
| : std::make_unique<std::unordered_map<std::string, std::string>>( | ||
| session_storage_map_); |
There was a problem hiding this comment.
There's no need to make a copy of the map here in the !storage_map.empty() case – you can keep using a const std::unordered_map<...>* pointer instead of a reference, along with an std::optional<std::unordered_map<...>> that is only filled when needed
|
|
||
| std::optional<node::webstorage::Storage*> DOMStorageAgent::getWebStorage( | ||
| bool is_local_storage) { | ||
| std::string var_name = is_local_storage ? "localStorage" : "sessionStorage"; |
There was a problem hiding this comment.
Using std::string_view is preferred when possible. In this case, you can take it a bit further and use FIXED_ONE_BYTE_STRING.
| ->Get(env_->context(), | ||
| v8::String::NewFromUtf8(env_->isolate(), var_name.c_str()) | ||
| .ToLocalChecked()) | ||
| .ToLocal(&web_storage_val) || |
There was a problem hiding this comment.
This feels like an exception that should be caught?
| return Array::New(env()->isolate(), values.data(), values.size()); | ||
| } | ||
|
|
||
| std::unordered_map<std::string, std::string> Storage::GetAll() { |
There was a problem hiding this comment.
| std::unordered_map<std::string, std::string> Storage::GetAll() { | |
| std::unordered_map<std::string, std::string> Storage::GetAll() const { |
?
| std::string value; | ||
| for (size_t i = 0; i < value_size; ++i) { | ||
| value.push_back(static_cast<char>(value_uint16[i])); | ||
| } |
There was a problem hiding this comment.
This is a very slow way to create strings, and completely broken for characters outside of the ISO-8859-1 range.
You'll want to look at the UTF-16-to-UTF-8 conversion functions exposed by simdutf and use those instead. (Alternatively, you could return std::u16string here, but that requires extra handling on the caller side then.)
3 participants
Web Storage data collection, which previously required explicitly firing events to send data to DevTools, is now performed automatically within Node.js.
Web Storage will appear in DevTools simply by specifying
--experimental-storage-inspection, without requiring any code changes.This feature relies on the changes introduced in the following change request, so it can be verified with the latest DevTools commit:
https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7274801