Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

flutteractionsbot
Copy link

@flutteractionsbot flutteractionsbot commented Dec 16, 2024

This pull request is created by automatic cherry pick workflow
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.

Issue Links:

flutter/flutter#160199
flutter/flutter#158093

Changelog Description:

Fix a regression that causes some images on the web to render blank.

Impact Description:

Some images are showing as a blank rectangle in the Stable release of Flutter Web.

Workaround:

No workaround as far as I know.

Risk:

What is the risk level of this cherry-pick?

  • Low
  • Medium
  • High

Test Coverage:

Are you confident that your fix is well-tested by automated tests?

  • Yes
  • No

Validation Steps:

Make sure there's an image rendered in the following app:

import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    var title = 'Web Images';

    return MaterialApp(
      title: title,
      home: Scaffold(
        appBar: AppBar(
          title: Text(title),
        ),
        body: Image.network(
          'https://picsum.photos/250?image=9',
          cacheHeight: 100,
        ),
      ),
    );
  }
}

A `CkImage` instance holds a reference to `ImageSource?`. When that `CkImage` gets cloned, the `ImageSource` instance becomes shared between the original `CkImage` and its new clone. Then when one of the `CkImage`s gets disposed of, it closes the shared `ImageSource` leaving other live `CkImage`s holding on to a closed `ImageSource`.

The quick solution to this is to have a ref count on the `ImageSource` to count how many `CkImage`s are referencing it. The `ImageSource` will only be closed if its ref count reaches 0.

Fixes flutter/flutter#160199
Fixes flutter/flutter#158093
@flutteractionsbot flutteractionsbot added the cp: review add the cp request to the review queue of release engineers label Dec 16, 2024
@flutteractionsbot
Copy link
Author

@mdebbar please fill out the PR description above, afterwards the release team will review this request.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Dec 16, 2024
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 9, 2025
@auto-submit auto-submit bot merged commit ce26d8b into flutter:flutter-3.27-candidate.0 Jan 9, 2025
29 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App cp: review add the cp request to the review queue of release engineers platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants