Bug 62662 – inspector/cookie-parser.html is a flaky crash

• Component: Web Inspector (Deprecated)
• Status: RESOLVED INVALID

[WebKit Review Bot]

[Chromium] inspector/cookie-parser.html is a flaky crash
Requested by dglazkov on #webkit.

[Dimitri Glazkov (Google)]

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20chromium.org&tests=inspector%2Fcookie-parser.html

[Adam Barth]

Dare I ask why the inspector has a cookie parser?

[Pavel Feldman]

(In reply to comment #2)
> Dare I ask why the inspector has a cookie parser?

[Pavel Feldman]

> (In reply to comment #2)
> > Dare I ask why the inspector has a cookie parser?

Feature: Show Cookies and Set-Cookie per request

Cookie parsing is a part of the browser / network stack, i.e. platform-specific. None of the platforms expose APIs for parsing the cookies (Set-Cookie in particular). It often is encapsulated in the network stack. So the options are

a)
- expose parsing APIs on platforms (not always possible)
- extend WebKit API to include parsed structures (no super good reason for that)
- extend Web Inspector protocol to accompany raw headers with parsed structures (also not such a good reason to increase API surface)
b) 
- check whether cookie parsing is conservative enough and do a 100 line snippet with good enough quality that would do that for us

(b) it is.

[Adam Barth]

Hopefully we use this parsing algorithm:

http://tools.ietf.org/html/rfc6265#section-5.2

:)

[Pavel Feldman]

(In reply to comment #5)
> Hopefully we use this parsing algorithm:
> http://tools.ietf.org/html/rfc6265#section-5.2
> :)

I think caseq@ ported it from Chromium with its deviations from the RFCs accepted by Mozilla and IE.

[Adam Barth]

> I think caseq@ ported it from Chromium with its deviations from the RFCs accepted by Mozilla and IE.

There shouldn't be any deviations from RFC 6265.  (That's the whole point of that spec.)  We're cleaning up any deviations that we discover to match that spec precisely.  If you know of any deviations, please let me know.

[Pavel Feldman]

> There shouldn't be any deviations from RFC 6265.  (That's the whole point of that spec.)  We're cleaning up any deviations that we discover to match that spec precisely.  If you know of any deviations, please let me know.

Oh, I was not familiar with the effort. I implied deviations from the old 2109/2965. Things like http://codesearch.google.com/#OAMlx_jo-ck/src/net/base/cookie_monster.cc&exact_package=chromium&q=offer%5Csan%5Csoption%5Csfor%5Csquoted&l=1829. We'll probably need to sync our port once the cleanup is complete.

[Adam Barth]

> Oh, I was not familiar with the effort. I implied deviations from the old 2109/2965.

Yes.  Those specs are not related to reality.

> Things like http://codesearch.google.com/#OAMlx_jo-ck/src/net/base/cookie_monster.cc&exact_package=chromium&q=offer%5Csan%5Csoption%5Csfor%5Csquoted&l=1829.

It's now clear how quoted strings should be handled.  I personally changed Firefox to match IE, Chrome, and RFC 6265.  Paul has promised to change CFNetwork to match as well.

[Adam Barth]

I've posted http://codereview.chromium.org/7155016 to remove these out-of-date and confusing comments.

[Pavel Feldman]

(In reply to comment #10)
> I've posted http://codereview.chromium.org/7155016 to remove these out-of-date and confusing comments.

Filed https://bugs.webkit.org/show_bug.cgi?id=62700 on inspector requiring its validation against the new rfc.

[Csaba Osztrogonác]

It isn't Chromium specific bug, because this test asserts 
on Qt in debug mode with NRWT, but passes with ORWT.

error log with r90729:

ASSERTION FAILED: enabled || !supportsProfiling()
../../../Source/WebCore/bindings/js/JSDOMWindowBase.cpp(126) : virtual bool WebCore::JSDOMWindowBase::supportsRichSourceInfo() const

[Balazs Kelemen]

(In reply to comment #12)
> It isn't Chromium specific bug, because this test asserts 
> on Qt in debug mode with NRWT, but passes with ORWT.
> 
> error log with r90729:
> 
> ASSERTION FAILED: enabled || !supportsProfiling()
> ../../../Source/WebCore/bindings/js/JSDOMWindowBase.cpp(126) : virtual bool WebCore::JSDOMWindowBase::supportsRichSourceInfo() const

After skip, the same assert failed with /inspector/cookie-resource-match-crash on Qt-Linux-x86_32-Debug: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Debug/r97849%20(19079)/inspector/cookie-resource-match-crash-log.txt

[Csaba Osztrogonác]

It is the first inspector test ... maybe skipping it too solve the problem ...

[Balazs Kelemen]

(In reply to comment #14)
> It is the first inspector test ... maybe skipping it too solve the problem ...

Let's try it. Skipped in http://trac.webkit.org/changeset/97862

[Csaba Osztrogonác]

One more test skipped by http://trac.webkit.org/changeset/98338

[Balazs Kelemen]

Trying a better approach to handle the bots by using test_expectations. Set the first one as CRASH+PASS: http://trac.webkit.org/changeset/98490.

[Brian Burg]

Closing as invalid, as this bug pertains to the old inspector UI and/or its tests.
Please file a new bug (https://www.webkit.org/new-inspector-bug) if the bug/feature/issue is still relevant to WebKit trunk.