Bug 173945

Summary: Async image decoding should be disabled for iBooks on tvOS
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: ImagesAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Said Abou-Hallawa 2017-06-28 16:28:02 PDT
The iBooks on tvOS is not a system application but it is an AppStore application. We need to disable async image decoding for iBooks on tvOS presently through WebKit.
Comment 1 Said Abou-Hallawa 2017-06-28 18:02:55 PDT
Created attachment 314077 [details]
Patch
Comment 2 Said Abou-Hallawa 2017-06-28 18:05:00 PDT
<rdar://problem/32516256>
Comment 3 Simon Fraser (smfr) 2017-06-28 18:23:46 PDT
Comment on attachment 314077 [details]
Patch

Let's wait until we've decided our general direction with async image decoding.
Comment 4 WebKit Commit Bot 2017-06-29 13:55:02 PDT
Comment on attachment 314077 [details]
Patch

Clearing flags on attachment: 314077

Committed r218961: <http://trac.webkit.org/changeset/218961>
Comment 5 WebKit Commit Bot 2017-06-29 13:55:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Darin Adler 2017-06-30 16:47:57 PDT
Comment on attachment 314077 [details]
Patch

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

> Source/WebCore/platform/graphics/BitmapImage.cpp:75
> +#if PLATFORM(IOS)
> +    if (!IOSApplication::isIBooks())
> +#endif
> +        m_allowLargeImageAsyncDecoding = settings.largeImageAsyncDecodingEnabled();

Wouldn’t it be slightly cleaner to set it to false when it is iOS iBooks instead of leaving it unset?
Comment 7 Said Abou-Hallawa 2017-07-03 12:07:20 PDT
Comment on attachment 314077 [details]
Patch

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

>> Source/WebCore/platform/graphics/BitmapImage.cpp:75
>> +        m_allowLargeImageAsyncDecoding = settings.largeImageAsyncDecodingEnabled();
> 
> Wouldn’t it be slightly cleaner to set it to false when it is iOS iBooks instead of leaving it unset?

Do you mean for readability because m_allowLargeImageAsyncDecoding is initialized to false in the header file? So you recommend the following?

#if PLATFORM(IOS)
    if (IOSApplication::isIBooks())
        m_allowLargeImageAsyncDecoding = false;
    else 
#endif
        m_allowLargeImageAsyncDecoding = settings.largeImageAsyncDecodingEnabled();

I will do this clean up.
Comment 8 Said Abou-Hallawa 2017-07-04 21:45:25 PDT
Committed r219125: <http://trac.webkit.org/changeset/219125>
Comment 9 Simon Fraser (smfr) 2017-07-05 13:32:01 PDT
Comment on attachment 314077 [details]
Patch

This should also do a linked-on-or-after check so that this workaround is not permanent.
Comment 10 Tim Horton 2017-07-05 13:51:55 PDT
(In reply to Simon Fraser (smfr) from comment #9)
> Comment on attachment 314077 [details]
> Patch
> 
> This should also do a linked-on-or-after check so that this workaround is
> not permanent.

And should JUST be about Storytime, not all iBooks.
Comment 11 Said Abou-Hallawa 2017-07-05 14:07:36 PDT
(In reply to Tim Horton from comment #10)
> (In reply to Simon Fraser (smfr) from comment #9)
> > Comment on attachment 314077 [details]
> > Patch
> > 
> > This should also do a linked-on-or-after check so that this workaround is
> > not permanent.
> 
> And should JUST be about Storytime, not all iBooks.

Can you explain what linked-on-or-after check is? It also very helpful if you give an example for how check is implemented.
Comment 12 Tim Horton 2017-07-05 14:08:28 PDT
(In reply to Said Abou-Hallawa from comment #11)
> (In reply to Tim Horton from comment #10)
> > (In reply to Simon Fraser (smfr) from comment #9)
> > > Comment on attachment 314077 [details]
> > > Patch
> > > 
> > > This should also do a linked-on-or-after check so that this workaround is
> > > not permanent.
> > 
> > And should JUST be about Storytime, not all iBooks.
> 
> Can you explain what linked-on-or-after check is? It also very helpful if
> you give an example for how check is implemented.

See all uses of dyld_get_program_sdk_version in the project.