Bug 170408 - Asynchronously decoded image frames should not be destroyed when switching encoded data buffers
Summary: Asynchronously decoded image frames should not be destroyed when switching en...
Status: RESOLVED DUPLICATE of bug 170640
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-03 11:38 PDT by Said Abou-Hallawa
Modified: 2017-04-10 10:35 PDT (History)
6 users (show)

See Also:

Attachments
Patch (23.56 KB, patch)
2017-04-03 12:34 PDT, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews112 for mac-elcapitan (369.58 KB, application/zip)
2017-04-03 13:34 PDT, Build Bot 		no flags Details
Patch (23.76 KB, patch)
2017-04-03 15:07 PDT, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2017-04-03 11:38:10 PDT 
When switching the encoded data buffers, it would be better if we could somehow tell the ImageDecoder to swap in the new contents without destroying anything. But right now the native ImageDecoder does not have a mechanism to do that so WebKit simply destroys all the decoded frames and then deletes the ImageDecoder itself. Then it sets the new data in the ImageSource which will recreate a new ImageDecoder.

For async image decoding, this aggressive flushing mechanism is not actually needed. The asynchronously decoded image frames are owned by WebKit. They are not referenced by the native ImageDecoder. Destroying them and re-decoding them again will generate exactly the same image frames. More importantly, destroying the decoded image frames, can cause flickering because the image can not be drawn till the second decode finish.
Comment 1 Said Abou-Hallawa 2017-04-03 11:39:30 PDT 
<rdar://problem/31211672>
Comment 2 Said Abou-Hallawa 2017-04-03 12:34:48 PDT 
Created attachment 306095 [details]
Patch
Comment 3 Simon Fraser (smfr) 2017-04-03 12:42:18 PDT 
Comment on attachment 306095 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=306095&action=review

> Source/WebCore/platform/graphics/Image.h:72
> +enum class ImageFrameDestroyScope {

Maybe ImageFramesToDestroy or ImageFrameDestroyBehavior

> Source/WebCore/platform/graphics/Image.h:77
> +    Shared

It's not clear what "Shared" means here. Shared with whom?
Comment 4 Build Bot 2017-04-03 13:34:49 PDT 
Comment on attachment 306095 [details]
Patch

Attachment 306095 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3466706

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2017-04-03 13:34:50 PDT 
Created attachment 306102 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 6 Said Abou-Hallawa 2017-04-03 15:07:37 PDT 
Created attachment 306122 [details]
Patch
Comment 7 Said Abou-Hallawa 2017-04-03 15:09:02 PDT 
Comment on attachment 306095 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=306095&action=review

>> Source/WebCore/platform/graphics/Image.h:72
>> +enum class ImageFrameDestroyScope {
> 
> Maybe ImageFramesToDestroy or ImageFrameDestroyBehavior

I renamed it to ImageFrameDestroyBehavior.

>> Source/WebCore/platform/graphics/Image.h:77
>> +    Shared
> 
> It's not clear what "Shared" means here. Shared with whom?

I renamed Shared to Synchronous.
Comment 8 Said Abou-Hallawa 2017-04-10 10:35:50 PDT 
If we prevent deleting the decoded frames of the images in the viewport then there is no need for this bug.

*** This bug has been marked as a duplicate of bug 170640 ***