Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CP-stable][web] Don't close image source too early #57221

Open
wants to merge 1 commit into
base: flutter-3.27-candidate.0
Choose a base branch
from

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants