Summary: | When disabling legacy private browsing for testing, change the SessionID back to what it was, not the defaultSessionID | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||||||||
Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | berto, cdumez, cgarcia, dbates, ews-watchlist, japhet, psaavedra, rwlbuis, webkit-bug-importer, youennf, zan | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 200050 | ||||||||||||||||
Attachments: |
|
Description
Alex Christensen
2019-09-04 16:50:58 PDT
Created attachment 378028 [details]
Patch
Comment on attachment 378028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378028&action=review > Source/WebCore/page/Page.h:940 > + const PAL::SessionID m_initialSessionID; It is not great to add another sessionID just for testing. We could replace Page::enableLegacyPrivateBrowsing by something like Page::setSessionIDForTesting. InjectedBundle would then store the real session ID and switch back and forth. That will unblock your work. Ultimately, we will want to do a full fix as started in bug 201475. Created attachment 378082 [details]
Patch
Created attachment 378083 [details]
Patch
This should not break the linux build, but it does. Could someone look into it? Created attachment 378112 [details]
WebKitGLibAPITests build fixes
WebKitGLibAPITests targets were throwing errors cause the WTF's config.h was used, which can't bring in the PAL export macros. Removing the WTF include flags avoids this, with WebKit's config.h getting used instead. This should be reworked further if TestWebKitAPI's config.h is ultimately preferred (which I'll ask about other people).
Created attachment 378138 [details]
Patch
Ready for review. This is currently blocking what I'm working on. Comment on attachment 378138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378138&action=review > Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.cpp:357 > + if (enabled) { Might be clearer to have one big if(enabled) instead of several enabled checks. Created attachment 378194 [details]
Patch
|