Bug 111883

Summary:

[WK2] Make it possible to reuse sandbox extensions

Product:

WebKit

Reporter:

Alexey Proskuryakov <ap>

Component:

WebKit2

Assignee:

Alexey Proskuryakov <ap>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, rniwa, sam, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

111917

Bug Blocks:

Attachments:

Description	Flags
proposed patch	andersca: review+
patch to land later	none
proposed patch	none

Alexey Proskuryakov

Reported 2013-03-08 12:17:11 PST

NetworkBlobRegistry holds sandbox extensions for as long as a blob exists, and the same blob could be loaded multiple times. Since an extension is revoked at the end of a load, this does not work.

Attachments
proposed patch (10.56 KB, patch) 2013-03-08 12:23 PST, Alexey Proskuryakov	andersca: review+	Details Formatted Diff Diff
patch to land later (11.37 KB, patch) 2013-03-08 22:21 PST, Alexey Proskuryakov	no flags	Details Formatted Diff Diff
proposed patch (13.51 KB, patch) 2013-03-09 10:43 PST, Alexey Proskuryakov	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2013-03-08 12:23:23 PST

Created attachment 192270 [details] proposed patch

Alexey Proskuryakov

Comment 2 2013-03-08 12:28:09 PST

Committed <http://trac.webkit.org/changeset/145254>.

Ryosuke Niwa

Comment 3 2013-03-08 16:36:49 PST

It seems like this patch caused a test failure: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fdom%2FWindow%2Fredirect-with-timer.html

WebKit Review Bot

Comment 4 2013-03-08 20:28:24 PST

Re-opened since this is blocked by bug 111917

Alexey Proskuryakov

Comment 5 2013-03-08 21:54:24 PST

Got it. The regression is caused by a bug in SandboxExtensionTracker that used to be hidden because we used to permanently leak the consumed extension in this case.

Alexey Proskuryakov

Comment 6 2013-03-08 22:21:07 PST

Created attachment 192335 [details] patch to land later Updated with build fix, removed an unneeded null check, and added an assertion that would have caught this condition earlier.

Alexey Proskuryakov

Comment 7 2013-03-09 10:43:41 PST

Created attachment 192346 [details] proposed patch It turned out that it makes much more sense to fix the pre-existing bug now, as that's easier to do with the new SandboxExtension functionality. The new patch also changes SandboxExtension to require manual balancing of consume and revoke calls. It's less error prone this way.

WebKit Review Bot

Comment 8 2013-03-11 08:54:06 PDT

Comment on attachment 192346 [details] proposed patch Clearing flags on attachment: 192346 Committed r145369: <http://trac.webkit.org/changeset/145369>

WebKit Review Bot

Comment 9 2013-03-11 08:54:10 PDT

All reviewed patches have been landed. Closing bug.

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