Bug 200669 – Remove SessionID default constructor

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[youenn fablet]

Instead use explicitly emptySessionID, which we can try to remove the use progressively.

[youenn fablet]

Created attachment 376159 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/54256381>

[Alex Christensen]

Comment on attachment 376159 [details]
Patch

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

> Source/WebCore/PAL/pal/SessionID.h:-35
> -    SessionID()

Maybe explicitly have = delete.

> Source/WebCore/platform/network/CookieRequestHeaderFieldProxy.h:67
> +        return { };

nullopt

> Source/WebKit/Shared/WebPageCreationParameters.h:100
> +    PAL::SessionID sessionID { PAL::SessionID::emptySessionID() };

I don't think this should be necessary.  Just go to the places where one of these is created and put the value in the constructor.

> Source/WebKit/UIProcess/API/APIPageConfiguration.cpp:52
> +    : m_sessionID(PAL::SessionID::emptySessionID())

ditto.

[youenn fablet]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 376159 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=376159&action=review
> 
> > Source/WebCore/PAL/pal/SessionID.h:-35
> > -    SessionID()
> 
> Maybe explicitly have = delete.

OK

> > Source/WebCore/platform/network/CookieRequestHeaderFieldProxy.h:67
> > +        return { };
> 
> nullopt
> 
> > Source/WebKit/Shared/WebPageCreationParameters.h:100
> > +    PAL::SessionID sessionID { PAL::SessionID::emptySessionID() };
> 
> I don't think this should be necessary.  Just go to the places where one of
> these is created and put the value in the constructor.

This allows making the patch smaller.
I plan to make a specific patch for this one.

> > Source/WebKit/UIProcess/API/APIPageConfiguration.cpp:52
> > +    : m_sessionID(PAL::SessionID::emptySessionID())
> 
> ditto.

https://bugs.webkit.org/show_bug.cgi?id=200670 removes APIPageConfiguration::m_sessionID.

[youenn fablet]

Created attachment 376252 [details]
Patch for landing

[Daniel Bates]

This patch largely undoes my cleanup in bug #184868, which I think is unfortunate and weird given:

1. you reviewed it
2. You don't mention undoing it or any reasons in the changelog.

Nothing to stop you from landing. Just wanted to point it out.

[youenn fablet]

(In reply to Daniel Bates from comment #6)
> This patch largely undoes my cleanup in bug #184868, which I think is
> unfortunate and weird given:
> 
> 1. you reviewed it
> 2. You don't mention undoing it or any reasons in the changelog.

With this patch, using decoder >> for sessionIDs is now needed.
I updated the decode method to use >> for consistency.

Agreed this is reverting part of bug 184868 patch but this should not change any behaviour.

I believe there is a trend to use >> more and more these days.
It is true we could decide on guidelines for decoders, whether to use >> or decode() by default for instance.
I personally tend to use >> for new decode methods.

[WebKit Commit Bot]

Comment on attachment 376252 [details]
Patch for landing

Clearing flags on attachment: 376252

Committed r248668: <https://trac.webkit.org/changeset/248668>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.