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: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=174541 | ||||||
Attachments: |
|
Description
Brady Eidson
2017-08-09 11:22:07 PDT
Created attachment 317726 [details]
Patch
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. (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. (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 on attachment 317726 [details] Patch Clearing flags on attachment: 317726 Committed r220475: <http://trac.webkit.org/changeset/220475> All reviewed patches have been landed. Closing bug. |