Bug 236111 – Fix App Privacy Report redirect attribution

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Kate Cheney]

Fix App Privacy Report redirect attribution

[Kate Cheney]

Created attachment 450841 [details]
Patch

[Sam Weinig]

Comment on attachment 450841 [details]
Patch

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

> Source/WebKit/ChangeLog:4
> +        Need a short description (OOPS!).
> +        Need the bug URL (OOPS!).

Looks like you forgot to fill this in.

> LayoutTests/ChangeLog:4
> +        Need a short description (OOPS!).
> +        Need the bug URL (OOPS!).

Same here.

[Brent Fulgham]

Comment on attachment 450841 [details]
Patch

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

And a test! I think this looks great, once you correct the ChangeLog. Thank you for jumping on this so quickly!

> Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp:372
>      auto oldInspectorInitiatorNodeIdentifier = inspectorInitiatorNodeIdentifier();

I think this is right, but I also suggest we clean up the constructors for CFNet and NSURLRequest.it may not fix anything, but I believe it is bad to ever have our WebKit flag out of sync with the NSURLRequest item.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:-624
> -#endif

Wry nice to simplify this!

[Kate Cheney]

Created attachment 450902 [details]
Patch for landing

[Kate Cheney]

(In reply to Sam Weinig from comment #2)
> Comment on attachment 450841 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450841&action=review
> 
> > Source/WebKit/ChangeLog:4
> > +        Need a short description (OOPS!).
> > +        Need the bug URL (OOPS!).
> 
> Looks like you forgot to fill this in.
> 
> > LayoutTests/ChangeLog:4
> > +        Need a short description (OOPS!).
> > +        Need the bug URL (OOPS!).
> 
> Same here.

Fixed! Thanks.

[Kate Cheney]

(In reply to Brent Fulgham from comment #3)
> Comment on attachment 450841 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450841&action=review
> 
> And a test! I think this looks great, once you correct the ChangeLog. Thank
> you for jumping on this so quickly!
> 
> > Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp:372
> >      auto oldInspectorInitiatorNodeIdentifier = inspectorInitiatorNodeIdentifier();
> 
> I think this is right, but I also suggest we clean up the constructors for
> CFNet and NSURLRequest.it may not fix anything, but I believe it is bad to
> ever have our WebKit flag out of sync with the NSURLRequest item.
> 

Yeah, agreed. I filed rdar://88490742 for this.

> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:-624
> > -#endif
> 
> Wry nice to simplify this!

[EWS]

Committed r289121 (246818@main): <https://commits.webkit.org/246818@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450902 [details].

[Radar WebKit Bug Importer]

<rdar://problem/88492251>