Bug 31067 - Chromium crashes when a plugin invokes the document.open function via NPAPI
Summary: Chromium crashes when a plugin invokes the document.open function via NPAPI
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-03 09:50 PST by Ananta Iyengar
Modified: 2009-11-10 14:22 PST (History)
3 users (show)

See Also:


Attachments
Initial patch with layout tests. (15.71 KB, patch)
2009-11-03 16:28 PST, Ananta Iyengar
abarth: review-
Details | Formatted Diff | Diff
Updated patch based on initial review comments (15.29 KB, patch)
2009-11-04 19:24 PST, Ananta Iyengar
no flags Details | Formatted Diff | Diff
Updated patch which now passes layout test runs on mac and windows. (16.28 KB, patch)
2009-11-05 12:28 PST, Ananta Iyengar
no flags Details | Formatted Diff | Diff
Updated patch with the document.open invocation from the plugin now done in npp_destroystream (15.53 KB, patch)
2009-11-06 12:39 PST, Ananta Iyengar
no flags Details | Formatted Diff | Diff
Patch (15.60 KB, patch)
2009-11-06 14:49 PST, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ananta Iyengar 2009-11-03 09:50:33 PST
This is a chromium only issue.
If a plugin invokes the document.open function on the document object via the NPN_Invoke function 
then the chrome renderer process crashes.

The crash occurs in the V8 bindings code.

The associated chromium bug is http://code.google.com/p/chromium/issues/detail?id=25703

I will submit a patch for this.
Comment 1 Ananta Iyengar 2009-11-03 13:19:08 PST
This crash occurs if the plugin invokes the NPN_Invoke API to open a document via document.open without an associated javascript context.

If the NPN_Invoke API is used to open a popup via window.open, it fails as well in Chromium.
Comment 2 Ananta Iyengar 2009-11-03 16:28:51 PST
Created attachment 42435 [details]
Initial patch with layout tests.
Comment 3 Adam Barth 2009-11-03 21:52:35 PST
Comment on attachment 42435 [details]
Initial patch with layout tests.

This looks great.  A couple minor issues:

 123     if (frame)
 124         htmlDocument->write(writeHelperGetString(args), frame->document());

We can call htmlDocument->write even when |frame| is null.  We can just pass a null second argument.

In your LayoutTest/ChangeLog, you have several copies of the change description.
Comment 4 Ananta Iyengar 2009-11-04 19:24:37 PST
Created attachment 42538 [details]
Updated patch based on initial review comments
Comment 5 Ananta Iyengar 2009-11-05 12:28:06 PST
Created attachment 42586 [details]
Updated patch which now passes layout test runs on mac and windows.
Comment 6 Ananta Iyengar 2009-11-06 12:39:16 PST
Created attachment 42669 [details]
Updated patch with the document.open invocation from the plugin now done in npp_destroystream
Comment 7 Adam Barth 2009-11-06 14:40:53 PST
Comment on attachment 42669 [details]
Updated patch with the document.open invocation from the plugin now done in npp_destroystream

This looks great.  One typo:

 132     if (frame)
 133         htmlDocument->writeln(writeHelperGetString(args), frame ? frame->document() : NULL);

We don't want the "if (frame)" here.  I can either fix this you on landing or we can use commit-queue if you upload a new patch.
Comment 8 Adam Barth 2009-11-06 14:49:32 PST
Created attachment 42672 [details]
Patch
Comment 9 WebKit Commit Bot 2009-11-06 15:00:55 PST
Comment on attachment 42672 [details]
Patch

Clearing flags on attachment: 42672

Committed r50607: <http://trac.webkit.org/changeset/50607>
Comment 10 WebKit Commit Bot 2009-11-06 15:01:01 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Kenneth Rohde Christiansen 2009-11-10 14:22:16 PST
Hi guys,

Running plugins/window-open.html with Qt WebKit does not open any window. Can you give me any hints on where to look in the code base for fixing this?

It basically calls invoke which uses a cross platform implementation. Our DRT also supports opening windows.