Performance issues due to multiple calls of the image_path_flush()

This is a simple and creative fix.

The only situation where this "might" fail is removing the image_path_flush($this->uri->value); on instantiation. Looking through the commit history, there is no mention of why they did it on entity creation as well, but it may happen if a crop is added to a field where an existing file entity is stored. This may happen in the case where someone is using a module to update a file with Media Entity File Replace or IMCE, or if the file entity is programmatically changed somehow (migration?). In these cases a particular style may already have a derivative, but I would guess it would be up to that module to clean them up? I'm grasping at straws honestly, but I want us to test those usecases.

As it is, using with focal point and none of these situations, this works amazingly well and is a vast improvment performance wise, ESPECIALLY on S3 and Pantheon Filesystems.

FYI I would have post a new relic stack trace, except that the improvements were so fast, new relic didn't keep any of the trace data. We went from 30+ seconds down to ~800ms with this fix on media creation with around 150 image styles and focal point installed.

I can confirm patch #2 improves the time to update a media entity (incl. manual crops, with image_widget_crop) vs. not using the patch.

I found that triggering the flush only for crop entity updates allows for an outdated image derivative to be served for this scenario:

1. Site using Crop API + Image Widget Crop.
2. An image style ImgSt configured to use only one effect (Manual Crop), using crop type CropA.
3. The entity with the image field is saved, but no crop CropA is defined (widget set to not require it)
4. The derivative for ImgSt is viewed/loaded (saves file)
5. The entity is saved again, this time applying a crop for CropA. This creates a crop entity for CropA + this image.
6. Loading the image derivative does not reflect the just-applied crop for CropA.
7. Re-save the entity (make sure to do any kind of change to crop CropA). This updates the crop entity for CropA + this image.
8. Loading the image derivative finally reflects the just-applied crop for CropA.

This effectively means, requiring $update == TRUE forces the user/editor to save a new crop twice (first creates it, second updates it) in order to see already-generated derivatives properly updated.

I'm attaching a patch that triggers the flush both for create and update scenarios of the crop entity. I just removed " && $update".

Note: I tried to reproduce the above scenario in a simplytest.me site, but couldn't. Since I don't have a way to verify that simplytest.me actually applied patch #2 (I definitely added it to the build), I decided to trust what I see in the site I'm working on which is consistent with what I read in the code.

I was running down a performance issue which this solves for the most part. But found another check that could be added in. Flushing image styles I'm pretty sure makes the assumption we are working with a uri that has a streamwrapper Drupal can work with. In a custom module, I have been making crops for images from a DAMS and have set the uri to an external URL (which i'm pretty sure is still valid as a uri) since I'm not actually working with a file in Drupal. When this get flushed it creates some weird paths to check and getting some errors from Drupal to to unlink, etc. So, I would propose checking for a valid stream wrapper, before flushing as well. It seemed appropriate to piggy back off of these patches, but if this should be a separate issue, let me know.

This is certainly an improvement worth making, but two things stand in its way:

1. Merge conflicts.
2. A performance fix like this will definitely need some kind of test coverage so that we don't accidentally regress it. It seems to me that a kernel test might be the best fit here?

Resolved merge conflicts and added a new kernel test CropImageStyleFlushTest

Wow, what a test! That's really thorough coverage and much appreciated. I have a couple of minor stylistic nits (feel free to ignore those if you don't agree with them), but my bigger concern is the tests being willing to bail out if the CI environment is misconfigured. That means the tests will not be reliable -- they might just be skipped even if something is broken. If the CI environment is misconfigured in such a way that it breaks these tests -- which is unlikely -- then that means we have to fix something about the CI environment, not the test.

Yea, those CI checks were left of from me debugging the test. I have removed them and applied your other suggestions.
Should be good now. Thanks!

No complaints here. Started a merge train for 8.x-2.x. Thanks!