Fix teardown in Web*LayerImpl
Created attachment 111648 [details] Patch
Since Web*LayerImpl inherit from LayerChromium that may call into its delegate from the destructor, we need to reset the delegate before calling that base destructor.
Comment on attachment 111648 [details] Patch Would it be possible to make some webkit_unit_tests that exercise creating/destroying WebLayers just to check for crashes like this?
Created attachment 111713 [details] Patch
Added tests. Also fixed a couple of missing setNeedsCommit indicated by the test.
Comment on attachment 111713 [details] Patch Attachment 111713 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10180240
Comment on attachment 111713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111713&action=review Yay tests that find bugs! R=me, few things to address while landing. > Source/WebKit/chromium/tests/WebLayerTest.cpp:16 > + * * Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * * Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following disclaimer > + * in the documentation and/or other materials provided with the > + * distribution. > + * * Neither the name of Google Inc. nor the names of its > + * contributors may be used to endorse or promote products derived from > + * this software without specific prior written permission. we prefer 2-clause nowadays, see Source/WebCore/LICENSE-APPLE (but put Google Inc on the top line and only include the current year) > Source/WebKit/chromium/tests/WebLayerTest.cpp:128 > + EXPECT_EQ(3, textureLayer.textureId()); use "3u" here or the compiler will whine about signed/unsigned mismatch
Created attachment 111824 [details] Patch
(In reply to comment #7) > (From update of attachment 111713 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=111713&action=review > > Yay tests that find bugs! R=me, few things to address while landing. > > > Source/WebKit/chromium/tests/WebLayerTest.cpp:16 > > + * * Redistributions of source code must retain the above copyright > > + * notice, this list of conditions and the following disclaimer. > > + * * Redistributions in binary form must reproduce the above > > + * copyright notice, this list of conditions and the following disclaimer > > + * in the documentation and/or other materials provided with the > > + * distribution. > > + * * Neither the name of Google Inc. nor the names of its > > + * contributors may be used to endorse or promote products derived from > > + * this software without specific prior written permission. > > we prefer 2-clause nowadays, see Source/WebCore/LICENSE-APPLE (but put Google Inc on the top line and only include the current year) Done. > > > Source/WebKit/chromium/tests/WebLayerTest.cpp:128 > > + EXPECT_EQ(3, textureLayer.textureId()); > > use "3u" here or the compiler will whine about signed/unsigned mismatch Done.
Created attachment 112027 [details] Patch
Previous patch was breaking LayerChromiumTest (testing the wrong assumption). Fixed now.
Comment on attachment 112027 [details] Patch Clearing flags on attachment: 112027 Committed r98393: <http://trac.webkit.org/changeset/98393>
All reviewed patches have been landed. Closing bug.
Reopenning to reland once http://codereview.chromium.org/8404008 is in (that should fix it).
After that lands make sure that Source/WebKit/chromium/DEPS points to a chromium rev including the change so it gets picked up on the build.webkit.org bots.
Created attachment 112633 [details] Patch
New patch with DEPS roll.
Comment on attachment 112633 [details] Patch R=me
Comment on attachment 112633 [details] Patch Clearing flags on attachment: 112633 Committed r98630: <http://trac.webkit.org/changeset/98630>