Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ struct OpData {
| kRecordBox | 11 | Record Box Model (border/padding/content) |
| kLinearGradient | 12 | Draw linear gradient |

For `kText` and `kImage`, the first integer argument is a platform paint
resource id returned by `NativePaintingContext`, not the Fragment/node id. The
Fragment id remains the owner id for events and lifecycle bookkeeping, while the
display list references the concrete text/image resource for the current content
snapshot.

#### Subtree Property Types (DisplayListSubtreePropertyOpType)

| Type | Value | Description |
Expand Down Expand Up @@ -348,14 +354,14 @@ The iOS platform parses DisplayList through **direct memory access**. The C++ la
break;
}
case DisplayListOpType::kText: {
auto text_id = [self nextContentInt];
auto text_id = [self nextContentInt]; // platform text resource id
auto box_index = [self nextContentInt];
// Get TextRenderer from LynxRendererContext to create LynxTextLayer
LynxTextLayer *layer = [[LynxTextLayer alloc] initWithLynxTextRenderer:...];
break;
}
case DisplayListOpType::kImage: {
auto image_id = [self nextContentInt];
auto image_id = [self nextContentInt]; // platform image resource id
auto box_index = [self nextContentInt];
// Create UIImageView and load image through LynxImageManager
UIImageView *imageView = [self createImageView];
Expand Down
19 changes: 11 additions & 8 deletions core/renderer/dom/fragment/fragment_behavior.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ class Fragment;
// Fragments are destroyed. This creates a use-after-free if subclasses try to
// access painting_context_ in their destructors.
//
// CORRECT APPROACH: Override OnElementDestroying() to release all platform
// resources (native views, text bundles, etc.). This method is called by
// Fragment::Destroy() while ElementManager and PaintingContext are still alive
// and all pointers remain valid.
// CORRECT APPROACH: Override OnElementDestroying() to release platform
// resources owned directly by this behavior. Display-list paint resources such
// as Text and Image are released by the platform display-list consumer after
// the active display list stops referencing their resource ids. This method is
// called by Fragment::Destroy() while ElementManager and PaintingContext are
// still alive and all pointers remain valid.
//
// Example:
// class MyBehavior : public FragmentBehavior {
Expand All @@ -56,10 +58,11 @@ class FragmentBehavior {

// Called by Fragment::Destroy() when the element is being destroyed.
//
// This is the ONLY safe place to release platform resources (native views,
// text bundles, image resources, etc.). At this point, ElementManager and
// PaintingContext are still alive, so painting_context_ and fragment_
// pointers remain valid.
// This is the ONLY safe place to release directly-owned platform resources.
// Display-list paint resources are released by the platform display-list
// consumer after their resource ids disappear from the active display list.
// At this point, ElementManager and PaintingContext are still alive, so
// painting_context_ and fragment_ pointers remain valid.
//
// DO NOT wait until the destructor to clean up resources - by then the
// PaintingContext may have already been destroyed, causing use-after-free
Expand Down
5 changes: 5 additions & 0 deletions core/renderer/dom/fragment/fragment_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,9 @@ TEST_F(FragmentTest, TestUpdateLayoutAndDefineBoxAndDrawImage) {
EXPECT_EQ(fragment.DefinePaddingBox(builder), 1);
EXPECT_EQ(fragment.DefineContentBox(builder), 2);

auto* image_behavior =
static_cast<ImageFragmentBehavior*>(fragment.behavior_.get());
image_behavior->image_resource_id_ = 123;
fragment.behavior_->OnDraw(builder);

DisplayList list = builder.Build();
Expand Down Expand Up @@ -810,6 +813,8 @@ TEST_F(FragmentTest, TestUpdateLayoutAndDefineBoxAndDrawImage) {
EXPECT_EQ(ops[3], static_cast<int32_t>(DisplayListOpType::kImage));
EXPECT_EQ(ints[6], 2);
EXPECT_EQ(ints[7], 0);
EXPECT_EQ(ints[8], 123);
EXPECT_EQ(ints[9], 2);
}

TEST_F(FragmentTest, TestCheckRootIfNeedClipBounds) {
Expand Down
7 changes: 5 additions & 2 deletions core/renderer/dom/fragment/image_fragment_behavior.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,19 @@ void ImageFragmentBehavior::OnUpdateLayout(
if (event_mask_ < 0) {
event_mask_ = ComputeEventMask();
}
painting_context_->CreateImage(
image_resource_id_ = painting_context_->CreateImageResource(
fragment_->id(), image_url_, layout_info.GetContentBoxWidth(),
layout_info.GetContentBoxHeight(), event_mask_);
fragment_->InvalidateForRedraw();
}
}

void ImageFragmentBehavior::OnDraw(DisplayListBuilder& display_list_builder) {
if (image_resource_id_ == kInvalidPlatformResourceId) {
return;
}
display_list_builder.DrawImage(
fragment()->id(), fragment()->DefineContentBox(display_list_builder));
image_resource_id_, fragment()->DefineContentBox(display_list_builder));
}

} // namespace lynx::tasm
1 change: 1 addition & 0 deletions core/renderer/dom/fragment/image_fragment_behavior.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class ImageFragmentBehavior : public FragmentBehavior {
int32_t ComputeEventMask() const;

base::String image_url_;
int32_t image_resource_id_{kInvalidPlatformResourceId};
// Cached event mask - computed lazily on first use, then never changes.
mutable int32_t event_mask_{-1}; // -1 means not yet computed
};
Expand Down
22 changes: 14 additions & 8 deletions core/renderer/dom/fragment/text_fragment_behavior.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ TextFragmentBehavior::TextFragmentBehavior(Fragment* fragment)
: FragmentBehavior(fragment) {}

void TextFragmentBehavior::OnElementDestroying() {
// Release platform resources while element is still accessible
if (painting_context_ && text_bundle_ != 0) {
painting_context_->DestroyTextBundle(fragment_->id());
text_bundle_ = 0;
}
text_bundle_ = 0;
last_text_bundle_ = 0;
text_resource_id_ = kInvalidPlatformResourceId;
}

void TextFragmentBehavior::CreatePlatformRenderer(
Expand All @@ -45,8 +43,14 @@ void TextFragmentBehavior::CreatePlatformRenderer(

void TextFragmentBehavior::OnUpdateLayout(
const LayoutInfoForDraw& layout_result) {
if (painting_context_ && fragment_) {
painting_context_->UpdateTextBundle(fragment_->id(), text_bundle_);
if (painting_context_ && fragment_ && text_bundle_ != last_text_bundle_) {
last_text_bundle_ = text_bundle_;
if (text_bundle_ == 0) {
text_resource_id_ = kInvalidPlatformResourceId;
} else {
text_resource_id_ =
painting_context_->CreateTextResource(fragment_->id(), text_bundle_);
}
}
DispatchLayoutEvent(layout_result);
}
Expand All @@ -58,7 +62,9 @@ void TextFragmentBehavior::OnDraw(DisplayListBuilder& builder) {
layout_info.layout_result.padding_[starlight::Direction::kTop],
layout_info.layout_result.size_.width_,
layout_info.layout_result.size_.height_);
builder.DrawText(fragment_->id(), fragment_->DefineBorderBox(builder));
if (text_resource_id_ != kInvalidPlatformResourceId) {
builder.DrawText(text_resource_id_, fragment_->DefineBorderBox(builder));
}
builder.End();
}

Expand Down
2 changes: 2 additions & 0 deletions core/renderer/dom/fragment/text_fragment_behavior.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class TextFragmentBehavior : public FragmentBehavior {

private:
intptr_t text_bundle_{0};
intptr_t last_text_bundle_{0};
int32_t text_resource_id_{kInvalidPlatformResourceId};
void DispatchLayoutEvent(const LayoutInfoForDraw& layout_result);
};
} // namespace lynx::tasm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,24 +490,25 @@ void NativePaintingCtxAndroid::ReconstructEventTargetTreeRecursively() {
});
}

void NativePaintingCtxAndroid::CreateImage(int id, base::String src,
float width, float height,
int32_t event_mask) {
int32_t NativePaintingCtxAndroid::CreateImageResource(int owner_id,
base::String src,
float width, float height,
int32_t event_mask) {
int32_t resource_id = NextPlatformResourceId();
if (view_manager_) {
view_manager_->CreateImage(id, src, width, height, event_mask);
view_manager_->CreateImageResource(resource_id, owner_id, src, width,
height, event_mask);
}
return resource_id;
}

void NativePaintingCtxAndroid::UpdateTextBundle(int id, intptr_t bundle) {
int32_t NativePaintingCtxAndroid::CreateTextResource(int owner_id,
intptr_t bundle) {
int32_t resource_id = NextPlatformResourceId();
if (view_manager_) {
view_manager_->UpdateTextBundle(id, bundle);
}
}

void NativePaintingCtxAndroid::DestroyTextBundle(int id) {
if (view_manager_) {
view_manager_->DestroyTextBundle(id);
view_manager_->CreateTextResource(resource_id, owner_id, bundle);
}
return resource_id;
}

void NativePaintingCtxAndroid::InsertListItemPaintingNode(int32_t list_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,10 @@ class NativePaintingCtxAndroid : public PaintingCtxPlatformImpl,
void UpdatePlatformEventBundle(int32_t id,
PlatformEventBundle bundle) override;

void CreateImage(int id, base::String src, float width, float height,
int32_t event_mask = 0) override;
int32_t CreateImageResource(int owner_id, base::String src, float width,
float height, int32_t event_mask = 0) override;

void UpdateTextBundle(int id, intptr_t bundle) override;

void DestroyTextBundle(int id) override;
int32_t CreateTextResource(int owner_id, intptr_t bundle) override;

void InsertListItemPaintingNode(int32_t list_id, int32_t child_id) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,11 @@ void PlatformRendererContext::DestroyPlatformRenderer(int32_t target) {
UnregisterPlatformRenderer(target);
}

void PlatformRendererContext::CreateImage(int32_t id, base::String src,
float width, float height,
int32_t event_mask) {
void PlatformRendererContext::CreateImageResource(int32_t resource_id,
int32_t owner_id,
base::String src, float width,
float height,
int32_t event_mask) {
base::android::ScopedLocalJavaRef<jobject> local_ref(java_ref_);
if (local_ref.IsNull()) {
return;
Expand All @@ -121,37 +123,41 @@ void PlatformRendererContext::CreateImage(int32_t id, base::String src,
auto j_src =
base::android::JNIConvertHelper::ConvertToJNIStringUTF(env, src.c_str());
Java_PlatformRendererContext_createImage(
env, local_ref.Get(), id, j_src.Get(), static_cast<int>(width),
static_cast<int>(height), static_cast<int>(event_mask));
env, local_ref.Get(), resource_id, owner_id, j_src.Get(),
static_cast<int>(width), static_cast<int>(height),
static_cast<int>(event_mask));
}

void PlatformRendererContext::DestroyImage(int32_t id) {
void PlatformRendererContext::DestroyImageResource(int32_t resource_id) {
base::android::ScopedLocalJavaRef<jobject> local_ref(java_ref_);
if (local_ref.IsNull()) {
return;
}
JNIEnv* env = base::android::AttachCurrentThread();
Java_PlatformRendererContext_destroyImage(env, local_ref.Get(), id);
Java_PlatformRendererContext_destroyImage(env, local_ref.Get(), resource_id);
}

void PlatformRendererContext::UpdateTextBundle(int32_t id,
intptr_t text_bundle) {
void PlatformRendererContext::CreateTextResource(int32_t resource_id,
int32_t owner_id,
intptr_t text_bundle) {
base::android::ScopedLocalJavaRef<jobject> local_ref(java_ref_);
if (local_ref.IsNull()) {
return;
}
JNIEnv* env = base::android::AttachCurrentThread();
Java_PlatformRendererContext_updateTextBundle(
env, local_ref.Get(), id, static_cast<jlong>(text_bundle));
env, local_ref.Get(), resource_id, owner_id,
static_cast<jlong>(text_bundle));
}

void PlatformRendererContext::DestroyTextBundle(int32_t id) {
void PlatformRendererContext::DestroyTextResource(int32_t resource_id) {
base::android::ScopedLocalJavaRef<jobject> local_ref(java_ref_);
if (local_ref.IsNull()) {
return;
}
JNIEnv* env = base::android::AttachCurrentThread();
Java_PlatformRendererContext_destroyTextBundle(env, local_ref.Get(), id);
Java_PlatformRendererContext_destroyTextBundle(env, local_ref.Get(),
resource_id);
}

void PlatformRendererContext::InsertListItemPaintingNode(int32_t list_sign,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ class PlatformRendererContext {
// Register/unregister PlatformRendererAndroid instances
void RegisterPlatformRenderer(int32_t id, PlatformRendererAndroid* renderer);
void UnregisterPlatformRenderer(int32_t id);
void CreateImage(int32_t id, base::String src, float width, float height,
int32_t event_mask = 0);
void DestroyImage(int32_t id);
void CreateImageResource(int32_t resource_id, int32_t owner_id,
base::String src, float width, float height,
int32_t event_mask = 0);
void DestroyImageResource(int32_t resource_id);

void UpdateTextBundle(int32_t id, intptr_t text_bundle);
void CreateTextResource(int32_t resource_id, int32_t owner_id,
intptr_t text_bundle);

void DestroyTextBundle(int32_t id);
void DestroyTextResource(int32_t resource_id);

void InsertListItemPaintingNode(int32_t list_sign, int32_t child_sign);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class NativePaintingCtxDarwin : public PaintingCtxPlatformImpl, public NativePai
void SetKeyframes(fml::RefPtr<PropBundle> keyframes_data) override {}

// NativePaintingContextDarwin do not need impl this interface.
virtual void HandleValidate(int tag) override{};
virtual void HandleValidate(int tag) override {}

std::unique_ptr<pub::Value> GetTextInfo(const std::string &content,
const pub::Value &info) override;
Expand Down Expand Up @@ -107,9 +107,7 @@ class NativePaintingCtxDarwin : public PaintingCtxPlatformImpl, public NativePai

void UpdateDisplayList(int id, DisplayList display_list) override;

void UpdateTextBundle(int id, intptr_t bundle) override;

void DestroyTextBundle(int id) override;
int32_t CreateTextResource(int owner_id, intptr_t bundle) override;

void InsertListItemPaintingNode(int32_t list_id, int32_t child_id) override;

Expand All @@ -123,8 +121,8 @@ class NativePaintingCtxDarwin : public PaintingCtxPlatformImpl, public NativePai

void UpdatePlatformEventBundle(int32_t id, PlatformEventBundle bundle) override;

void CreateImage(int id, base::String src, float width, float height,
int32_t event_mask = 0) override;
int32_t CreateImageResource(int owner_id, base::String src, float width, float height,
int32_t event_mask = 0) override;

#pragma endregion // NativePaintingContext

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,23 +234,17 @@
});
}

void NativePaintingCtxDarwin::UpdateTextBundle(int id, intptr_t bundle) {
Enqueue([ref = platform_ref_, id, bundle]() {
int32_t NativePaintingCtxDarwin::CreateTextResource(int owner_id, intptr_t bundle) {
int32_t resource_id = NextPlatformResourceId();
Enqueue([ref = platform_ref_, resource_id, owner_id, bundle]() {
auto darwin_ref = std::static_pointer_cast<NativePaintingCtxPlatformDarwinRef>(ref);
if (darwin_ref) {
[darwin_ref->GetRendererContext() updateTextBundle:id
withBundle:reinterpret_cast<void *>(bundle)];
}
});
}

void NativePaintingCtxDarwin::DestroyTextBundle(int id) {
Enqueue([ref = platform_ref_, id]() {
auto darwin_ref = std::static_pointer_cast<NativePaintingCtxPlatformDarwinRef>(ref);
if (darwin_ref) {
[darwin_ref->GetRendererContext() destroyTextBundle:id];
[darwin_ref->GetRendererContext() createTextResource:resource_id
ownerID:owner_id
withBundle:reinterpret_cast<void *>(bundle)];
}
});
return resource_id;
}

void NativePaintingCtxDarwin::InsertListItemPaintingNode(int32_t list_id, int32_t child_id) {}
Expand Down Expand Up @@ -281,16 +275,19 @@
});
}

void NativePaintingCtxDarwin::CreateImage(int id, base::String src, float width, float height,
int32_t event_mask) {
int32_t NativePaintingCtxDarwin::CreateImageResource(int owner_id, base::String src, float width,
float height, int32_t event_mask) {
int32_t resource_id = NextPlatformResourceId();
LynxURL *sourceUrl = [[LynxURL alloc] init];
sourceUrl.url = [[NSURL alloc] initWithString:[[NSString alloc] initWithUTF8String:src.c_str()]];
sourceUrl.imageSize = CGSizeMake(width, height);

[context_->GetRendererContext() createImageManager:id
[context_->GetRendererContext() createImageManager:resource_id
ownerID:owner_id
withSourceURL:sourceUrl
andPlaceholderURL:nil
eventMask:event_mask];
return resource_id;
}

template <typename F>
Expand Down
Loading
Loading