Skip to content

Commit

Permalink
Use new UnguessableToken::Deserialize method in most places
Browse files Browse the repository at this point in the history
This CL moves existing calls of UnguessableToken::Deserialize
to use the replacement method that returns an absl::optional
instead of an UnguessableToken directly. Once all calls have
been moved, the previous method will be removed and the new
method will be renamed to finish replacement.

There are three remaining Deserialize calls that we will replace
in follow-up CLs:
 - `UnguessableTokenAndroid::FromJavaUnguessableToken` and
   `PlatformHandleInternal::FromJavaUnguessableToken`, since we
   will want these to return an absl::optional also

 - `UnguessableToken::CreateForTesting`, since we won't update
   that to return an absl::optional and will instead just crash
   if high == 0 and low == 0 get passed.

Bug: 1402549
Change-Id: Ibea1abd9e11716f11fb127a6c42c6bfc756b79d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4135540
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Andrew Williams <awillia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096142}
  • Loading branch information
recvfrom authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent deaa509 commit c07db02
Show file tree
Hide file tree
Showing 31 changed files with 213 additions and 131 deletions.
5 changes: 3 additions & 2 deletions base/json/values_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ absl::optional<UnguessableToken> ValueToUnguessableToken(const Value& value) {
UnguessableTokenRepresentation repr;
if (!HexStringToSpan(value.GetString(), repr.buffer))
return absl::nullopt;
auto token = UnguessableToken::Deserialize(repr.field.high, repr.field.low);
if (token.is_empty()) {
absl::optional<base::UnguessableToken> token =
UnguessableToken::Deserialize2(repr.field.high, repr.field.low);
if (!token.has_value()) {
return absl::nullopt;
}
return token;
Expand Down
7 changes: 4 additions & 3 deletions base/metrics/field_trial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,13 @@ bool DeserializeGUIDFromStringPieces(StringPiece first,
if (!StringToUint64(first, &high) || !StringToUint64(second, &low))
return false;

UnguessableToken token = UnguessableToken::Deserialize(high, low);
if (token.is_empty()) {
absl::optional<UnguessableToken> token =
UnguessableToken::Deserialize2(high, low);
if (!token.has_value()) {
return false;
}

*guid = std::move(token);
*guid = token.value();
return true;
}
#endif // !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_IOS)
Expand Down
7 changes: 6 additions & 1 deletion chrome/browser/ash/dbus/vm/vm_permission_service_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ base::UnguessableToken TokenFromString(const std::string& str) {
count++;
});

return base::UnguessableToken::Deserialize(high, low);
absl::optional<base::UnguessableToken> token =
base::UnguessableToken::Deserialize2(high, low);
if (!token.has_value()) {
return base::UnguessableToken();
}
return token.value();
}

} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ base::UnguessableToken GetUnguessableToken(base::StringPiece str) {
absl::optional<base::Token> token = base::Token::FromString(str);
DCHECK(token.has_value());

return base::UnguessableToken::Deserialize(token->high(), token->low());
absl::optional<base::UnguessableToken> unguessable_token =
base::UnguessableToken::Deserialize2(token->high(), token->low());
return unguessable_token.value();
}

// Class used to wait for InstanceRegistry events.
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/paint_preview/paint_preview_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,10 @@ class PaintPreviewBrowserTest
const gfx::Size& size = gfx::Size(1, 1)) {
base::ScopedAllowBlockingForTesting scoped_blocking;

auto it = recording_map->find(base::UnguessableToken::Deserialize(
frame_proto.embedding_token_high(), frame_proto.embedding_token_low()));
auto it = recording_map->find(
base::UnguessableToken::Deserialize2(frame_proto.embedding_token_high(),
frame_proto.embedding_token_low())
.value());
ASSERT_NE(it, recording_map->end());

absl::optional<SkpResult> result = std::move(it->second).Deserialize();
Expand Down
7 changes: 6 additions & 1 deletion chrome/browser/serial/serial_chooser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ base::UnguessableToken DecodeToken(base::StringPiece input) {
}

const uint64_t* data = reinterpret_cast<const uint64_t*>(buffer.data());
return base::UnguessableToken::Deserialize(data[0], data[1]);
absl::optional<base::UnguessableToken> token =
base::UnguessableToken::Deserialize2(data[0], data[1]);
if (!token.has_value()) {
return base::UnguessableToken();
}
return token.value();
}

base::Value PortInfoToValue(const device::mojom::SerialPortInfo& port) {
Expand Down
8 changes: 6 additions & 2 deletions components/mirroring/service/video_capture_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,19 @@ namespace {

// Required by mojom::VideoCaptureHost interface. Can be any nonzero value.
const base::UnguessableToken& DeviceId() {
// TODO(https://crbug.com/1406986): Investigate whether there's a better way
// to accomplish this (without using UnguessableToken::Deserialize).
static const base::UnguessableToken device_id(
base::UnguessableToken::Deserialize(1, 1));
base::UnguessableToken::Deserialize2(1, 1).value());
return device_id;
}

// Required by mojom::VideoCaptureHost interface. Can be any nonzero value.
const base::UnguessableToken& SessionId() {
// TODO(https://crbug.com/1406986): Investigate whether there's a better way
// to accomplish this (without using UnguessableToken::Deserialize).
static const base::UnguessableToken session_id(
base::UnguessableToken::Deserialize(1, 1));
base::UnguessableToken::Deserialize2(1, 1).value());
return session_id;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,10 @@ TEST_P(PaintPreviewBaseServiceTest, CaptureMainFrame) {
EXPECT_TRUE(result->proto.has_root_frame());
EXPECT_EQ(result->proto.subframes_size(), 0);
EXPECT_TRUE(result->proto.root_frame().is_main_frame());
auto token = base::UnguessableToken::Deserialize(
result->proto.root_frame().embedding_token_high(),
result->proto.root_frame().embedding_token_low());
auto token = base::UnguessableToken::Deserialize2(
result->proto.root_frame().embedding_token_high(),
result->proto.root_frame().embedding_token_low())
.value();
switch (GetParam()) {
case RecordingPersistence::kFileSystem: {
#if BUILDFLAG(IS_WIN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,10 @@ TEST_P(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) {
EXPECT_EQ(returned_guid, expected_guid);
EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);

auto token = base::UnguessableToken::Deserialize(
result->proto.root_frame().embedding_token_high(),
result->proto.root_frame().embedding_token_low());
auto token = base::UnguessableToken::Deserialize2(
result->proto.root_frame().embedding_token_high(),
result->proto.root_frame().embedding_token_low())
.value();
EXPECT_NE(token, base::UnguessableToken::Null());

// The token for the main frame is set internally since the render frame
Expand Down
26 changes: 17 additions & 9 deletions components/paint_preview/common/recording_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,18 @@ RecordingMap RecordingMapFromPaintPreviewProto(const PaintPreviewProto& proto) {
if (!root_frame_recording.IsValid())
return {};

auto root_frame_embedding_token = base::UnguessableToken::Deserialize(
proto.root_frame().embedding_token_high(),
proto.root_frame().embedding_token_low());
if (root_frame_embedding_token.is_empty()) {
absl::optional<base::UnguessableToken> root_frame_embedding_token =
base::UnguessableToken::Deserialize2(
proto.root_frame().embedding_token_high(),
proto.root_frame().embedding_token_low());
// TODO(https://crbug.com/1406995): Investigate whether a deserialization
// failure can actually occur here and if it can, add a comment discussing
// how this can happen.
if (!root_frame_embedding_token.has_value()) {
return {};
}

entries.emplace_back(std::move(root_frame_embedding_token),
entries.emplace_back(root_frame_embedding_token.value(),
std::move(root_frame_recording));

for (const auto& subframe : proto.subframes()) {
Expand All @@ -78,13 +82,17 @@ RecordingMap RecordingMapFromPaintPreviewProto(const PaintPreviewProto& proto) {
if (!frame_recording.IsValid())
continue;

auto subframe_embedding_token = base::UnguessableToken::Deserialize(
subframe.embedding_token_high(), subframe.embedding_token_low());
if (subframe_embedding_token.is_empty()) {
absl::optional<base::UnguessableToken> subframe_embedding_token =
base::UnguessableToken::Deserialize2(subframe.embedding_token_high(),
subframe.embedding_token_low());
// TODO(https://crbug.com/1406995): Investigate whether a deserialization
// failure can actually occur here and if it can, add a comment discussing
// how this can happen.
if (!subframe_embedding_token.has_value()) {
continue;
}

entries.emplace_back(std::move(subframe_embedding_token),
entries.emplace_back(subframe_embedding_token.value(),
std::move(frame_recording));
}

Expand Down
12 changes: 8 additions & 4 deletions components/paint_preview/player/player_compositor_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,17 @@ namespace {

std::pair<base::UnguessableToken, std::unique_ptr<HitTester>> BuildHitTester(
const PaintPreviewFrameProto& proto) {
auto embedding_token = base::UnguessableToken::Deserialize(
proto.embedding_token_high(), proto.embedding_token_low());
if (embedding_token.is_empty()) {
absl::optional<base::UnguessableToken> embedding_token =
base::UnguessableToken::Deserialize2(proto.embedding_token_high(),
proto.embedding_token_low());
// TODO(https://crbug.com/1406995): Investigate whether a deserialization
// failure can actually occur here and if it can, add a comment discussing how
// this can happen.
if (!embedding_token.has_value()) {
embedding_token = base::UnguessableToken::Create();
}
std::pair<base::UnguessableToken, std::unique_ptr<HitTester>> out(
std::move(embedding_token), std::make_unique<HitTester>());
embedding_token.value(), std::make_unique<HitTester>());
out.second->Build(proto);
return out;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@ absl::optional<PaintPreviewFrame> BuildFrame(
continue;

mojom::SubframeClipRect rect;
rect.frame_guid = base::UnguessableToken::Deserialize(
id_pair.embedding_token_high(), id_pair.embedding_token_low());
if (rect.frame_guid.is_empty()) {
absl::optional<base::UnguessableToken> maybe_deserialized_token =
base::UnguessableToken::Deserialize2(id_pair.embedding_token_high(),
id_pair.embedding_token_low());
if (!maybe_deserialized_token.has_value()) {
continue;
}
rect.frame_guid = maybe_deserialized_token.value();

rect.clip_rect = rect_it->second;

Expand Down Expand Up @@ -198,10 +200,11 @@ void PaintPreviewCompositorImpl::BeginSeparatedFrameComposite(
}

DCHECK(paint_preview.has_value());
response->root_frame_guid = base::UnguessableToken::Deserialize(
paint_preview->root_frame().embedding_token_high(),
paint_preview->root_frame().embedding_token_low());
if (response->root_frame_guid.is_empty()) {
absl::optional<base::UnguessableToken> embedding_token =
base::UnguessableToken::Deserialize2(
paint_preview->root_frame().embedding_token_high(),
paint_preview->root_frame().embedding_token_low());
if (!embedding_token.has_value()) {
DVLOG(1) << "No valid root frame guid";
// Cannot send a null token over mojo. This will be ignored downstream.
response->root_frame_guid = base::UnguessableToken::Create();
Expand All @@ -210,6 +213,7 @@ void PaintPreviewCompositorImpl::BeginSeparatedFrameComposite(
std::move(response));
return;
}
response->root_frame_guid = embedding_token.value();
auto frames = DeserializeAllFrames(std::move(request->recording_map));

// Adding the root frame must succeed.
Expand Down Expand Up @@ -290,17 +294,19 @@ void PaintPreviewCompositorImpl::BeginMainFrameComposite(
}

DCHECK(paint_preview.has_value());
base::UnguessableToken root_frame_guid = base::UnguessableToken::Deserialize(
paint_preview->root_frame().embedding_token_high(),
paint_preview->root_frame().embedding_token_low());
if (root_frame_guid.is_empty()) {
absl::optional<base::UnguessableToken> maybe_root_frame_guid =
base::UnguessableToken::Deserialize2(
paint_preview->root_frame().embedding_token_high(),
paint_preview->root_frame().embedding_token_low());
if (!maybe_root_frame_guid.has_value()) {
DVLOG(1) << "No valid root frame guid";
response->root_frame_guid = base::UnguessableToken::Create();
std::move(callback).Run(mojom::PaintPreviewCompositor::
BeginCompositeStatus::kDeserializingFailure,
std::move(response));
return;
}
base::UnguessableToken root_frame_guid = maybe_root_frame_guid.value();

base::flat_map<base::UnguessableToken, sk_sp<SkPicture>> loaded_frames;
RecordingMap recording_map = std::move(request->recording_map);
Expand Down Expand Up @@ -328,7 +334,7 @@ void PaintPreviewCompositorImpl::BeginMainFrameComposite(
paint_preview->root_frame().has_scroll_offset_y()
? paint_preview->root_frame().scroll_offset_y()
: 0);
response->frames.insert({std::move(root_frame_guid), std::move(frame_data)});
response->frames.insert({root_frame_guid, std::move(frame_data)});

std::move(callback).Run(
subframes_failed
Expand Down Expand Up @@ -380,11 +386,13 @@ bool PaintPreviewCompositorImpl::AddFrame(
const PaintPreviewFrameProto& frame_proto,
const base::flat_map<base::UnguessableToken, SkpResult>& skp_map,
mojom::PaintPreviewBeginCompositeResponsePtr* response) {
base::UnguessableToken guid = base::UnguessableToken::Deserialize(
frame_proto.embedding_token_high(), frame_proto.embedding_token_low());
if (guid.is_empty()) {
absl::optional<base::UnguessableToken> maybe_guid =
base::UnguessableToken::Deserialize2(frame_proto.embedding_token_high(),
frame_proto.embedding_token_low());
if (!maybe_guid.has_value()) {
return false;
}
base::UnguessableToken guid = maybe_guid.value();

absl::optional<PaintPreviewFrame> maybe_frame =
BuildFrame(guid, frame_proto, skp_map);
Expand All @@ -403,7 +411,7 @@ bool PaintPreviewCompositorImpl::AddFrame(
frame_data->subframes.push_back(subframe_clip_rect.Clone());

(*response)->frames.insert({guid, std::move(frame_data)});
frames_.insert({std::move(guid), std::move(maybe_frame.value())});
frames_.insert({guid, std::move(maybe_frame.value())});
return true;
}

Expand Down Expand Up @@ -439,11 +447,13 @@ sk_sp<SkPicture> PaintPreviewCompositorImpl::DeserializeFrameRecursive(
base::flat_map<base::UnguessableToken, sk_sp<SkPicture>>* loaded_frames,
RecordingMap* recording_map,
bool* subframe_failed) {
auto frame_guid = base::UnguessableToken::Deserialize(
frame_proto.embedding_token_high(), frame_proto.embedding_token_low());
if (frame_guid.is_empty()) {
absl::optional<base::UnguessableToken> maybe_frame_guid =
base::UnguessableToken::Deserialize2(frame_proto.embedding_token_high(),
frame_proto.embedding_token_low());
if (!maybe_frame_guid.has_value()) {
return nullptr;
}
base::UnguessableToken frame_guid = maybe_frame_guid.value();
TRACE_EVENT1("paint_preview",
"PaintPreviewCompositorImpl::DeserializeFrameRecursive",
"frame_guid", frame_guid.ToString());
Expand All @@ -460,13 +470,16 @@ sk_sp<SkPicture> PaintPreviewCompositorImpl::DeserializeFrameRecursive(

*subframe_failed = false;
for (const auto& id_pair : frame_proto.content_id_to_embedding_tokens()) {
auto subframe_embedding_token = base::UnguessableToken::Deserialize(
id_pair.embedding_token_high(), id_pair.embedding_token_low());
absl::optional<base::UnguessableToken> maybe_subframe_embedding_token =
base::UnguessableToken::Deserialize2(id_pair.embedding_token_high(),
id_pair.embedding_token_low());

if (subframe_embedding_token.is_empty()) {
if (!maybe_subframe_embedding_token.has_value()) {
DVLOG(1) << "Subframe has invalid embedding token";
continue;
}
base::UnguessableToken subframe_embedding_token =
maybe_subframe_embedding_token.value();

// This subframe is already loaded.
if (loaded_frames->contains(subframe_embedding_token)) {
Expand All @@ -479,13 +492,14 @@ sk_sp<SkPicture> PaintPreviewCompositorImpl::DeserializeFrameRecursive(
auto subframe_proto_it =
base::ranges::find(subframes, subframe_embedding_token,
[](const PaintPreviewFrameProto& frame_proto) {
auto token = base::UnguessableToken::Deserialize(
frame_proto.embedding_token_high(),
frame_proto.embedding_token_low());
if (token.is_empty()) {
absl::optional<base::UnguessableToken> token =
base::UnguessableToken::Deserialize2(
frame_proto.embedding_token_high(),
frame_proto.embedding_token_low());
if (!token.has_value()) {
return base::UnguessableToken::Create();
}
return token;
return token.value();
});
if (subframe_proto_it == subframes.end()) {
DVLOG(1) << "Frame embeds subframe that does not exist: "
Expand Down
8 changes: 4 additions & 4 deletions components/user_notes/storage/user_note_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ UserNoteMetadataSnapshot UserNoteDatabase::GetNoteMetadataForUrls(
!base::HexStringToUInt64(string_piece.substr(16, 16), &low)) {
continue;
}
base::UnguessableToken token =
base::UnguessableToken::Deserialize(high, low);
if (token.is_empty()) {
absl::optional<base::UnguessableToken> token =
base::UnguessableToken::Deserialize2(high, low);
if (!token.has_value()) {
continue;
}

Expand All @@ -115,7 +115,7 @@ UserNoteMetadataSnapshot UserNoteDatabase::GetNoteMetadataForUrls(

auto metadata = std::make_unique<UserNoteMetadata>(
creation_date, modification_date, /*min_note_version=*/1);
metadata_snapshot.AddEntry(url, token, std::move(metadata));
metadata_snapshot.AddEntry(url, token.value(), std::move(metadata));
}
}

Expand Down
7 changes: 4 additions & 3 deletions content/browser/android/dialog_overlay_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@ static jlong JNI_DialogOverlayImpl_Init(JNIEnv* env,
jboolean power_efficient) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

base::UnguessableToken token = base::UnguessableToken::Deserialize(high, low);
if (token.is_empty()) {
absl::optional<base::UnguessableToken> token =
base::UnguessableToken::Deserialize2(high, low);
if (!token.has_value()) {
return 0;
}

RenderFrameHostImpl* rfhi =
content::RenderFrameHostImpl::FromOverlayRoutingToken(std::move(token));
content::RenderFrameHostImpl::FromOverlayRoutingToken(token.value());

if (!rfhi)
return 0;
Expand Down

0 comments on commit c07db02

Please sign in to comment.