Bug 175391 - Teach ScriptExecutionContexts about their SessionID
Summary: Teach ScriptExecutionContexts about their SessionID
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-09 11:22 PDT by Brady Eidson
Modified: 2017-08-09 13:26 PDT (History)
8 users (show)

See Also:

Attachments
Patch (31.72 KB, patch)
2017-08-09 11:57 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
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 Brady Eidson 2017-08-09 11:22:07 PDT 
Teach ScriptExecutionContexts about their SessionID

Especially needed to expose SessionIDs to workers.
Comment 1 Brady Eidson 2017-08-09 11:57:19 PDT 
Created attachment 317726 [details]
Patch
Comment 2 Andy Estes 2017-08-09 12:05:10 PDT 
Comment on attachment 317726 [details]
Patch

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

> Source/WebCore/page/SessionID.cpp:61
> +SessionID SessionID::isolatedCopy() const
> +{
> +    return SessionID { m_sessionID };
> +}

Seems like you could just do a regular copy-by-value in WorkerThreadStartupData instead of adding this, since there's no actual isolated copying going on here.
Comment 3 Brady Eidson 2017-08-09 12:56:14 PDT 
(In reply to Andy Estes from comment #2)
> Comment on attachment 317726 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317726&action=review
> 
> > Source/WebCore/page/SessionID.cpp:61
> > +SessionID SessionID::isolatedCopy() const
> > +{
> > +    return SessionID { m_sessionID };
> > +}
> 
> Seems like you could just do a regular copy-by-value in
> WorkerThreadStartupData instead of adding this, since there's no actual
> isolated copying going on here.

I much prefer future proofing (future resisting?) any class that is passed cross thread.

By having isolatedCopy() in the SessionID.h header, it suggests to anybody adding any future arguments that they need to account for them.
Comment 4 Brady Eidson 2017-08-09 13:04:45 PDT 
(In reply to Brady Eidson from comment #3)
> (In reply to Andy Estes from comment #2)
> > Comment on attachment 317726 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=317726&action=review
> > 
> > > Source/WebCore/page/SessionID.cpp:61
> > > +SessionID SessionID::isolatedCopy() const
> > > +{
> > > +    return SessionID { m_sessionID };
> > > +}
> > 
> > Seems like you could just do a regular copy-by-value in
> > WorkerThreadStartupData instead of adding this, since there's no actual
> > isolated copying going on here.
> 
> I much prefer future proofing (future resisting?) any class that is passed
> cross thread.
> 
> By having isolatedCopy() in the SessionID.h header, it suggests to anybody
> adding any future arguments that they need to account for them.

s/future arguments/future members/
Comment 5 WebKit Commit Bot 2017-08-09 13:26:01 PDT 
Comment on attachment 317726 [details]
Patch

Clearing flags on attachment: 317726

Committed r220475: <http://trac.webkit.org/changeset/220475>
Comment 6 WebKit Commit Bot 2017-08-09 13:26:03 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2017-08-09 13:26:44 PDT 
<rdar://problem/33809118>