Bug 236111 - Fix App Privacy Report redirect attribution
Summary: Fix App Privacy Report redirect attribution
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks: 238159
  Show dependency treegraph
 
Reported: 2022-02-03 17:18 PST by Kate Cheney
Modified: 2022-03-21 15:54 PDT (History)
3 users (show)

See Also:


Attachments
Patch (8.96 KB, patch)
2022-02-03 17:21 PST, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch for landing (12.27 KB, patch)
2022-02-04 08:34 PST, Kate Cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kate Cheney 2022-02-03 17:18:41 PST
Fix App Privacy Report redirect attribution
Comment 1 Kate Cheney 2022-02-03 17:21:42 PST
Created attachment 450841 [details]
Patch
Comment 2 Sam Weinig 2022-02-03 17:36:30 PST
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.
Comment 3 Brent Fulgham 2022-02-03 18:12:34 PST
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!
Comment 4 Kate Cheney 2022-02-04 08:34:12 PST
Created attachment 450902 [details]
Patch for landing
Comment 5 Kate Cheney 2022-02-04 08:34:37 PST
(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.
Comment 6 Kate Cheney 2022-02-04 08:37:41 PST
(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!
Comment 7 EWS 2022-02-04 09:07:47 PST
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].
Comment 8 Radar WebKit Bug Importer 2022-02-04 09:08:23 PST
<rdar://problem/88492251>