Bug 139453 – REGRESSION (r173272): When open PDF from Safari in iBooks, title is replaced to “QuickLookPDF-s72DbgAU-1”

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Antti Koivisto]

Bug in CFNetwork code path

[Antti Koivisto]

Created attachment 242947 [details]
patch

[Antti Koivisto]

rdar://problem/19052192

[Pratik Solanki]

Comment on attachment 242947 [details]
patch

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

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=139453

Can you add the radar link here as well.

[Antti Koivisto]

https://trac.webkit.org/r177031

[Darin Adler]

Comment on attachment 242947 [details]
patch

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

> Source/WebCore/platform/network/cf/ResourceResponseCFNet.cpp:133
> -    RetainPtr<CFStringRef> suggestedFilename = adoptCF(CFURLResponseCopySuggestedFilename(m_cfResponse.get()));
> +    RetainPtr<CFStringRef> suggestedFilename = adoptCF(CFURLResponseCopySuggestedFilename(cfURLResponse()));
>      return suggestedFilename.get();

I think that code like this reads better without a local RetainPtr variable:

    return adoptCF(CFURLResponseCopySuggestedFilename(cfURLResponse())).get();

In fact, I would write this:

    CFURLResponseRef underlyingResponse = cfURLResponse();
    if (!underlyingResponse)
        return String();
    return adoptCF(CFURLResponseCopySuggestedFilename(underlyingResponse)).get();

[Antti Koivisto]

>     CFURLResponseRef underlyingResponse = cfURLResponse();
>     if (!underlyingResponse)
>         return String();
>     return adoptCF(CFURLResponseCopySuggestedFilename(underlyingResponse)).get();

I wouldn't call it 'underlying' though as it doesn't describe the role of a synthesized CFURLResponse very well.