[Region Capture][cropTo][#8] Move crop-ID prod out of Element
Crop-ID production should be done in the browser process. Rationale:
1. It removes concerns of interference by a malicious application
that could otherwise be forcing collisions of crop-ID.
2. When cropTo() is called, validation of that a crop-ID (a) exists
and (b) belongs to the current browsing-context will ultimately
be done in the browser process.
This CL performs the first step by moving the production of the crop-ID
out of Element and into MediaDevices (where produceCropId is exposed
to JS). The next CL(s?) will move the production further, to the
browser process, with MediaDevices holding a Resolver that can finally
fulfill its Promise when the browser process responds with a newly
minted crop-ID.
Drive-by: Make GetRegionCaptureCropId() return a const-ptr.
Bug: 1247761
Change-Id: Ie0aa515584a7ef312bee2fa740f0f318c9b13646
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3253017
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Jordan Bayles <jophba@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937320}
diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
index 70801f79..6d2f8fc 100644
--- a/third_party/blink/renderer/core/dom/element.cc
+++ b/third_party/blink/renderer/core/dom/element.cc
@@ -32,7 +32,6 @@
#include <memory>
#include <utility>
-#include "base/guid.h"
#include "cc/input/snap_selection_strategy.h"
#include "third_party/blink/public/common/privacy_budget/identifiability_metric_builder.h"
#include "third_party/blink/public/common/privacy_budget/identifiability_study_settings.h"
@@ -3624,24 +3623,14 @@
layout_object->Layer()->SetNeedsRepaint();
}
-base::GUID Element::MarkWithRegionCaptureCropId() {
+void Element::SetRegionCaptureCropId(
+ std::unique_ptr<RegionCaptureCropId> crop_id) {
ElementRareData& rare_data = EnsureElementRareData();
- const RegionCaptureCropId* const region_capture_id =
- rare_data.GetRegionCaptureCropId();
- if (region_capture_id) {
- // Convert the pre-existing crop-ID back into its GUID-form.
- // This is the less efficient string-based form that we avoid propagating
- // through the rendering pipeline, but which the Web-application expects.
- return TokenToGUID(region_capture_id->value());
- }
-
- // Produce a new crop-ID.
- const base::GUID guid = base::GUID::GenerateRandomV4();
+ CHECK(!rare_data.GetRegionCaptureCropId());
// Propagate efficient form through the rendering pipeline.
- rare_data.SetRegionCaptureCropId(
- std::make_unique<RegionCaptureCropId>(GUIDToToken(guid)));
+ rare_data.SetRegionCaptureCropId(std::move(crop_id));
// The crop ID needs to be propagated to the paint system by the time that
// capture begins. The API requires the implementation to propagate the
@@ -3649,11 +3638,9 @@
if (GetLayoutObject()) {
GetLayoutObject()->SetShouldDoFullPaintInvalidation();
}
-
- return guid;
}
-RegionCaptureCropId* Element::GetRegionCaptureCropId() const {
+const RegionCaptureCropId* Element::GetRegionCaptureCropId() const {
return HasRareData() ? GetElementRareData()->GetRegionCaptureCropId()
: nullptr;
}
diff --git a/third_party/blink/renderer/core/dom/element.h b/third_party/blink/renderer/core/dom/element.h
index f7a16f5..487030f 100644
--- a/third_party/blink/renderer/core/dom/element.h
+++ b/third_party/blink/renderer/core/dom/element.h
@@ -27,7 +27,6 @@
#include "base/dcheck_is_on.h"
#include "base/gtest_prod_util.h"
-#include "base/guid.h"
#include "third_party/blink/public/common/input/pointer_id.h"
#include "third_party/blink/public/common/metrics/document_update_reason.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_typedefs.h"
@@ -567,15 +566,17 @@
void SetNeedsCompositingUpdate();
- // Generates a unique crop ID and returns its base::GUID representation.
- // Not all element types have a region capture crop id, however using it here
+ // Assigns a crop-ID to the element. It's an error to try to call this
+ // if the element already has a crop-ID attached (even if attempting to
+ // set the same crop-ID again).
+ //
+ // Not all element types have a region capture crop-ID, however using it here
// allows access to the element rare data struct. Currently, once an element
- // is marked for region capture it cannot be unmarked, and repeated calls to
- // this API will return the same token.
- base::GUID MarkWithRegionCaptureCropId();
+ // is marked for region capture it cannot be unmarked.
+ void SetRegionCaptureCropId(std::unique_ptr<RegionCaptureCropId> crop_id);
- // Returns nullptr if not marked for capture.
- RegionCaptureCropId* GetRegionCaptureCropId() const;
+ // Returns a pointer to the crop-ID if one was set; nullptr otherwise.
+ const RegionCaptureCropId* GetRegionCaptureCropId() const;
ShadowRoot* attachShadow(const ShadowRootInit*, ExceptionState&);
diff --git a/third_party/blink/renderer/core/dom/element_rare_data.h b/third_party/blink/renderer/core/dom/element_rare_data.h
index 460e06a..e21186b0 100644
--- a/third_party/blink/renderer/core/dom/element_rare_data.h
+++ b/third_party/blink/renderer/core/dom/element_rare_data.h
@@ -149,11 +149,18 @@
return element_internals_;
}
- RegionCaptureCropId* GetRegionCaptureCropId() const {
+ // Returns the crop-ID if one was set, or nullptr otherwise.
+ const RegionCaptureCropId* GetRegionCaptureCropId() const {
return region_capture_crop_id_.get();
}
- void SetRegionCaptureCropId(std::unique_ptr<RegionCaptureCropId> value) {
- region_capture_crop_id_ = std::move(value);
+
+ // Sets a crop-ID on the item. Must be called at most once. Cannot be used
+ // to unset a previously set crop-ID.
+ void SetRegionCaptureCropId(std::unique_ptr<RegionCaptureCropId> crop_id) {
+ DCHECK(!GetRegionCaptureCropId());
+ DCHECK(crop_id);
+ DCHECK(!crop_id->value().is_zero());
+ region_capture_crop_id_ = std::move(crop_id);
}
void SetStyleShouldForceLegacyLayout(bool force) {
diff --git a/third_party/blink/renderer/modules/mediastream/media_devices.cc b/third_party/blink/renderer/modules/mediastream/media_devices.cc
index 198ebcd..7c95810 100644
--- a/third_party/blink/renderer/modules/mediastream/media_devices.cc
+++ b/third_party/blink/renderer/modules/mediastream/media_devices.cc
@@ -350,13 +350,30 @@
element_union->IsHTMLDivElement()
? static_cast<Element*>(element_union->GetAsHTMLDivElement())
: static_cast<Element*>(element_union->GetAsHTMLIFrameElement());
- const base::GUID crop_id = element->MarkWithRegionCaptureCropId();
- DCHECK(crop_id.is_valid());
- // Guaranteed because the empty Token is not a valid UUID-v4.
- DCHECK_NE(crop_id, blink::TokenToGUID(base::Token()));
+
+ const RegionCaptureCropId* const old_crop_id =
+ element->GetRegionCaptureCropId();
+ if (old_crop_id) { // produceCropId() previously called on this element.
+ DCHECK(!old_crop_id->value().is_zero());
+ resolver->Resolve(WTF::String(
+ blink::TokenToGUID(old_crop_id->value()).AsLowercaseString()));
+ return promise;
+ }
+
+ // TODO(crbug.com/1247761): Produce the crop-ID in the browser process,
+ // where it's free from interference by potential malicious applications
+ // trying to produce collisions. Also, we need it there in order to
+ // validate IDs provided to cropTo() for (1) validity and (2) association
+ // with the current browsing context.
+ const base::GUID new_crop_id = base::GUID::GenerateRandomV4();
+ element->SetRegionCaptureCropId(
+ std::make_unique<RegionCaptureCropId>(blink::GUIDToToken(new_crop_id)));
// TODO(crbug.com/1247761): Delay resolution until ack from Viz received.
- const std::string& serialized_crop_id = crop_id.AsLowercaseString();
+ // Multiple calls to produceCropId() will be handled by returning distinct
+ // Promises which are all fulfilled (in sequence) when the first one is
+ // fulfilled.
+ const std::string& serialized_crop_id = new_crop_id.AsLowercaseString();
resolver->Resolve(
WTF::String(serialized_crop_id.c_str(), serialized_crop_id.length()));
return promise;
