Allow existing Document wrappers during V8 snapshot deserialization [chromium/src : main]
0 views
Skip to first unread message
Nate Chapin (Gerrit)
Jun 24, 2026, 4:39:21 PM (5 days ago) Jun 24
to Evan Luo, Kentaro Hara, Raphael Kubo da Costa, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Attention needed from Evan Luo
Nate Chapin voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nate Chapin
| Code-Review | +1 |
1 comment
File third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc
Line 243, Patchset 7 (Latest):
static_cast<void>(Nate Chapin . unresolved
`std::ignore` is more of the norm in blink - any particular reason we should prefer `static_cast<void>` here?
Related details
Attention is currently required from:
- Evan Luo
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Evan Luo (Gerrit)
Jun 24, 2026, 5:11:37 PM (5 days ago) Jun 24
to Nate Chapin, Kentaro Hara, Raphael Kubo da Costa, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Evan Luo voted and added 1 comment![]( "Open in Gerrit")
Votes added by Evan Luo
| Commit-Queue | +2 |
1 comment
File third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc
`std::ignore` is more of the norm in blink - any particular reason we should prefer `static_cast<void>` here?
Evan Luo
Nope no particular reason, thanks for the pointer!
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jun 24, 2026, 6:33:59 PM (5 days ago) Jun 24
to Evan Luo, Nate Chapin, Kentaro Hara, Raphael Kubo da Costa, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
7 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc
Insertions: 3, Deletions: 2.
@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.h"
#include <array>
+#include <tuple>
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_context_snapshot.h"
@@ -240,10 +241,10 @@
// The return value is discarded because whether SetWrapperInInlineStorage
// associates a new wrapper (true) or retains an existing wrapper (false),
// the Document is guaranteed to hold a valid wrapper in inline storage.
- static_cast<void>(
+ std::ignore =
DOMDataStore::SetWrapperInInlineStorage</*entered_context=*/false>(
deserializer_data->isolate, deserializer_data->html_document,
- V8HTMLDocument::GetWrapperTypeInfo(), holder));
+ V8HTMLDocument::GetWrapperTypeInfo(), holder);
}
// We only care for WrapperTypeInfo and do not supply an actual instance of
```
```
The name of the file: third_party/blink/renderer/core/frame/local_frame_back_forward_cache_test.cc
Insertions: 4, Deletions: 2.
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <tuple>
+
#include "base/run_loop.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
@@ -193,8 +195,8 @@
// Access WindowProxy before UpdateDocument() installs the next page's
// Document. Verify that reinitializing a context against an existing Document
// whose inline storage already contains a wrapper completes cleanly.
- static_cast<void>(frame->WindowProxy(
- DOMWrapperWorld::MainWorld(script_state->GetIsolate())));
+ std::ignore = frame->WindowProxy(
+ DOMWrapperWorld::MainWorld(script_state->GetIsolate()));
}
} // namespace blink
```
Change information
Commit message:
Allow existing Document wrappers during V8 snapshot deserialization
WindowProxy::InitializeIfNeeded() can force snapshot context creation
against the existing Document. If its inline storage already contains a
wrapper, SetWrapperInInlineStorage safely retains it and returns false.
Removing CHECK(result) is safe because it leaves the existing valid
wrapper intact and allows snapshot deserialization to complete normally
without crashing.
Bug: 511262076
Change-Id: I1b87419d15ba330ebf2d5b4c5e533549b817a411
Reviewed-by: Nate Chapin <jap...@chromium.org>
Commit-Queue: Evan Luo <eva...@google.com>
Cr-Commit-Position: refs/heads/main@{#1652004}
Files:
- M third_party/blink/renderer/bindings/modules/v8/v8_context_snapshot_impl.cc
- M third_party/blink/renderer/core/frame/local_frame_back_forward_cache_test.cc
Change size: S
Delta: 2 files changed, 40 insertions(+), 2 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Nate Chapin
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages