Bug 60274

Summary:

[Windows WebKit2] Use cookies set in WebKit1

Product:

WebKit

Reporter:

Jessie Berlin <jberlin>

Component:

WebKit2

Assignee:

Jessie Berlin <jberlin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aroben, beidson, darin, jberlin, jhoneycutt, sam, sfalken

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

Windows 7

Bug Depends on:

60445, 60660

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Path supplementing the one already reviewed that should fix the crashes on the WK2 bots	none
Patch with a better fix for the crash and with a fix for a loose end	sfalken: review+
Patch with a better fix for the crash and with a fix for a loose end (Take 2)	darin: review+

Jessie Berlin

Reported 2011-05-05 09:03:32 PDT

<rdar://problem/8927952>

Attachments
Patch (915.48 KB, patch) 2011-05-05 12:56 PDT, Jessie Berlin	no flags	Details Formatted Diff Diff
Path supplementing the one already reviewed that should fix the crashes on the WK2 bots (1.13 KB, patch) 2011-05-11 16:19 PDT, Jessie Berlin	no flags	Details Formatted Diff Diff
Patch with a better fix for the crash and with a fix for a loose end (897.37 KB, patch) 2011-05-12 10:21 PDT, Jessie Berlin	sfalken: review+	Details Formatted Diff Diff
Patch with a better fix for the crash and with a fix for a loose end (Take 2) (deleted) 2011-05-12 11:53 PDT, Jessie Berlin	darin: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Jessie Berlin

Comment 1 2011-05-05 12:56:42 PDT

Created attachment 92453 [details] Patch

Jessie Berlin

Comment 2 2011-05-07 17:57:40 PDT

Comment on attachment 92453 [details] Patch Committed in http://trac.webkit.org/changeset/86016

Jessie Berlin

Comment 3 2011-05-07 18:14:45 PDT

This change caused crashes on the WK2 Windows bots, so I had sheriffbot roll it out in http://trac.webkit.org/changeset/86018

Jessie Berlin

Comment 4 2011-05-11 16:19:54 PDT

Created attachment 93207 [details] Path supplementing the one already reviewed that should fix the crashes on the WK2 bots

Jessie Berlin

Comment 5 2011-05-11 17:14:33 PDT

Comment on attachment 93207 [details] Path supplementing the one already reviewed that should fix the crashes on the WK2 bots Recommitted the patch with this fix in http://trac.webkit.org/changeset/86285. And it passes on the tests! http://build.webkit.org/results/Windows%207%20Release%20(WebKit2%20Tests)/r86285%20(6745)/results.html

Jessie Berlin

Comment 6 2011-05-12 09:30:57 PDT

There are a few outstanding issues that need addressing.

Jessie Berlin

Comment 7 2011-05-12 10:21:59 PDT

Created attachment 93299 [details] Patch with a better fix for the crash and with a fix for a loose end

Adam Roben (:aroben)

Comment 8 2011-05-12 11:17:11 PDT

Comment on attachment 93299 [details] Patch with a better fix for the crash and with a fix for a loose end View in context: https://bugs.webkit.org/attachment.cgi?id=93299&action=review > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:45 > + , serializedDefaultStorageSessionIsValid(false) I don't think this extra member is needed. You can still encode and decode the boolean just using local variables. There's no need to pass the boolean around in the parameters struct itself.

Jessie Berlin

Comment 9 2011-05-12 11:53:11 PDT

Created attachment 93318 [details] Patch with a better fix for the crash and with a fix for a loose end (Take 2)

Darin Adler

Comment 10 2011-05-12 11:54:43 PDT

Comment on attachment 93318 [details] Patch with a better fix for the crash and with a fix for a loose end (Take 2) View in context: https://bugs.webkit.org/attachment.cgi?id=93318&action=review > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:166 > - if (parameters.serializedDefaultStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > + bool hasStorageSession = false; > + if (!decoder->decode(hasStorageSession)) > + return false; > + if (hasStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > return false; What zeroes out parameters.serializedDefaultStorageSession?

Adam Roben (:aroben)

Comment 11 2011-05-12 11:55:41 PDT

Comment on attachment 93318 [details] Patch with a better fix for the crash and with a fix for a loose end (Take 2) View in context: https://bugs.webkit.org/attachment.cgi?id=93318&action=review > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:89 > + CFDataRef storageSession = serializedDefaultStorageSession.get(); > + encoder->encodeBool(storageSession); > + if (storageSession) > + CoreIPC::encode(encoder, storageSession); You could probably get away without the local storageSession variable. But having it seems fine.

Jessie Berlin

Comment 12 2011-05-12 12:06:26 PDT

(In reply to comment #10) > (From update of attachment 93318 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=93318&action=review > > > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:166 > > - if (parameters.serializedDefaultStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > > + bool hasStorageSession = false; > > + if (!decoder->decode(hasStorageSession)) > > + return false; > > + if (hasStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > > return false; > > What zeroes out parameters.serializedDefaultStorageSession? It is a RetainPtr, which is automatically initialized to 0 when the WebProcessCreationParameters is constructed in the Web Process.

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