Bug 111883 - [WK2] Make it possible to reuse sandbox extensions
Summary: [WK2] Make it possible to reuse sandbox extensions
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
Depends on: 111917
  Show dependency treegraph
Reported: 2013-03-08 12:17 PST by Alexey Proskuryakov
Modified: 2013-03-11 08:54 PDT (History)
4 users (show)

See Also:

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

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 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.
Comment 1 Alexey Proskuryakov 2013-03-08 12:23:23 PST
Created attachment 192270 [details]
proposed patch
Comment 2 Alexey Proskuryakov 2013-03-08 12:28:09 PST
Committed <http://trac.webkit.org/changeset/145254>.
Comment 4 WebKit Review Bot 2013-03-08 20:28:24 PST
Re-opened since this is blocked by bug 111917
Comment 5 Alexey Proskuryakov 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.
Comment 6 Alexey Proskuryakov 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.
Comment 7 Alexey Proskuryakov 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.
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2013-03-11 08:54:10 PDT
All reviewed patches have been landed.  Closing bug.