| Summary: | Crash in imageLoadEventTimerFired after adoptNode used on <img>, seen with inspector, which uses adoptNode | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||
| Component: | Images | Assignee: | Darin Adler <darin> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Mac | ||||||||
| OS: | OS X 10.5 | ||||||||
| Attachments: |
|
||||||||
|
Description
Darin Adler
2009-03-16 10:19:10 PDT
Created attachment 28652 [details]
patch
Created attachment 28656 [details]
patch, this time with a regression test
Comment on attachment 28656 [details]
patch, this time with a regression test
+ member functions. Got rid of pointless boolean bit fields that don't make the
+ object any smaller, since 3 bits and 3 bytes both cost 4 bytes.
On PowerPC, bool is still 4 bytes - I don't think that we should regress PPC, especially when that's easy to avoid.
+ m_dispatchingList = m_dispatchSoonList;
+ m_dispatchSoonList.clear();
A swap would be more efficient than copying.
Index: LayoutTests/fast/dom/HTMLImageElement/resources/image-load-subframe.html
===================================================================
--- LayoutTests/fast/dom/HTMLImageElement/resources/image-load-subframe.html (revision 0)
+++ LayoutTests/fast/dom/HTMLImageElement/resources/image-load-subframe.html (revision 0)
@@ -0,0 +1 @@
+<body><img src="blue_rect.jpg"></body>
With a data: URL, the test should be easier to read. It isn't great that a failure in this test is detected as a crash in another one.
r=me
(In reply to comment #4) > On PowerPC, bool is still 4 bytes - I don't think that we should regress PPC, > especially when that's easy to avoid. Really? I didn't know that. I had asked about this earlier but I guess it was in person, not on an email list. > + m_dispatchingList = m_dispatchSoonList; > + m_dispatchSoonList.clear(); > > A swap would be more efficient than copying. Sure, I'll do that. > With a data: URL, the test should be easier to read. It isn't great that a > failure in this test is detected as a crash in another one. I was unable to make the test that crashed with the old code with a data URL. I tried for days. I was unable to make a test that would crash within the test. Adding code to wait for the image to load (and hence to wait for the crash) made the bug go away. I'd be happy to improve the test, but I was unable to. (In reply to comment #5) > I was unable to make the test that crashed with the old code with a data URL. The difficulty was primarily because the data URL didn't have permission to load an image that wasn't also a data URL. Maybe someone else can figure out the trick. (In reply to comment #5) > Really? I didn't know that. I had asked about this earlier but I guess it was > in person, not on an email list. I made a test program that printed sizeof(bool), and it was 4 bytes (12 bytes for a struct with three of those). But perhaps WebKit uses some build setting that changes the default? (In reply to comment #6) > The difficulty was primarily because the data URL didn't have permission to > load an image that wasn't also a data URL. Maybe someone else can figure out > the trick. One possible trick is to use javascript:'' URLs, which share security origin with the opener. (In reply to comment #8) > > The difficulty was primarily because the data URL didn't have permission to > > load an image that wasn't also a data URL. Maybe someone else can figure out > > the trick. > > One possible trick is to use javascript:'' URLs, which share security origin > with the opener. I'm not going to spend more time on this now, but I'll keep that in mind next time I need to test something like this or if I decided to come back and refine this test. |