Bug 175391

Summary:

Teach ScriptExecutionContexts about their SessionID

Product:

WebKit

Reporter:

Brady Eidson <beidson>

Component:

WebCore Misc.

Assignee:

Brady Eidson <beidson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aestes, buildbot, cdumez, commit-queue, dbates, esprehn+autocc, kangil.han, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=174541

Attachments:

Description	Flags
Patch	none

Brady Eidson

Reported 2017-08-09 11:22:07 PDT

Teach ScriptExecutionContexts about their SessionID Especially needed to expose SessionIDs to workers.

Attachments
Patch (31.72 KB, patch) 2017-08-09 11:57 PDT, Brady Eidson	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2017-08-09 11:57:19 PDT

Created attachment 317726 [details] Patch

Andy Estes

Comment 2 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.

Brady Eidson

Comment 3 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.

Brady Eidson

Comment 4 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/

WebKit Commit Bot

Comment 5 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>

WebKit Commit Bot

Comment 6 2017-08-09 13:26:03 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 7 2017-08-09 13:26:44 PDT

<rdar://problem/33809118>

Note You need to log in before you can comment on or make changes to this bug.