Summary: | [Windows] Initial Cookie Accept Policy not set on Deserialized Storage Session | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jessie Berlin <jberlin> | ||||
Component: | WebKit2 | Assignee: | Jessie Berlin <jberlin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | darin, jberlin, sfalken | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | Windows 7 | ||||||
Attachments: |
|
Description
Jessie Berlin
2011-05-22 16:46:48 PDT
Created attachment 94357 [details]
Patch
Comment on attachment 94357 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94357&action=review > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:144 > + WebCookieManager::shared().setHTTPCookieAcceptPolicy(parameters.initialHTTPCookieAcceptPolicy); Moving this one line up seems like the bug fix. The other changes seem OK, but I don’t see why they have to go along with this bug fix in the same patch. They could be done separately before or after. > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:147 > + // By using the default storage session that came from the UI Process, the Web Process > + // automatically use the same the URL Cache as UI Process. I suggest not capitalizing terms such as “web process”. Also, the second line should read: // automatically uses the same URL cache as the UI process. r=me whether you decide to break this up in pieces or not (In reply to comment #2) > (From update of attachment 94357 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=94357&action=review > > > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:144 > > + WebCookieManager::shared().setHTTPCookieAcceptPolicy(parameters.initialHTTPCookieAcceptPolicy); > > Moving this one line up seems like the bug fix. The other changes seem OK, but I don’t see why they have to go along with this bug fix in the same patch. They could be done separately before or after. It is true they could be done separately, but I without them the code with my change is harder to read (which was my motivation for making those changes now). I think I am going to keep them (per your other comment) for that reason. > > > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:147 > > + // By using the default storage session that came from the UI Process, the Web Process > > + // automatically use the same the URL Cache as UI Process. > > I suggest not capitalizing terms such as “web process”. Also, the second line should read: > > // automatically uses the same URL cache as the UI process. Fixed. Thanks for the review! Comment on attachment 94357 [details] Patch Committed in http://trac.webkit.org/changeset/87077 |