31108 2009-11-04 01:05:33 -0800 [V8] REGRESSION: Pause on exception is broken 2009-11-05 10:36:03 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 yurys jianli abarth dglazkov jianli levin pfeldman oldest_to_newest 160366 0 42462 yurys 2009-11-04 01:05:33 -0800 Created attachment 42462 A test page What steps will reproduce the problem? 1. Open the attached page. 2. Open devtools and make sure 'Pause on exception' is on(by default it is). 3. Click 'Test' button. What is the expected output? Execution is paused inside f(); What do you see instead? Exception is recorded in console but script execution is not paused in the Scripts panel and you cannot inspect the call stack. This is a regression introduced by http://trac.webkit.org/changeset/48610 160430 1 yurys 2009-11-04 06:40:14 -0800 I think we should change MessageCallback added to v8 by V8Proxy. Currently the callback is V8ConsoleMessage::handler but it seems that it just needs to dispatch the exception message to the right handler(page's document or worker context). 160459 2 abarth 2009-11-04 09:14:05 -0800 (In reply to comment #1) > I think we should change MessageCallback added to v8 by V8Proxy. Currently the > callback is V8ConsoleMessage::handler but it seems that it just needs to > dispatch the exception message to the right handler(page's document or worker > context). I'm not too familiar with how the devtools hook in here. Can you upload a patch that does the above so we can see what it looks like in code? 160467 3 pfeldman 2009-11-04 09:33:34 -0800 (In reply to comment #2) > (In reply to comment #1) > > I think we should change MessageCallback added to v8 by V8Proxy. Currently the > > callback is V8ConsoleMessage::handler but it seems that it just needs to > > dispatch the exception message to the right handler(page's document or worker > > context). > > I'm not too familiar with how the devtools hook in here. Can you upload a > patch that does the above so we can see what it looks like in code? We bisected regression to this change and it does not seem to justify well enough why it decided to temporarily set verbose to false in this very place of the code. The suggested fix is to basically revert trunk/WebCore/bindings/v8/V8AbstractEventListener.cpp change introduced in http://trac.webkit.org/changeset/48610. In fact that is why comment was initially a follow up to that bug. Note that there is jam's change that happened in between, but i think it should stay. 160469 4 dglazkov 2009-11-04 09:36:01 -0800 Looks like if jianli broke it, he should be the one fixing this, right? 160526 5 42501 jianli 2009-11-04 11:20:56 -0800 Created attachment 42501 Proposed Patch 160541 6 42501 pfeldman 2009-11-04 11:58:32 -0800 Comment on attachment 42501 Proposed Patch Thanks for fixing this! We should make "stop on exceptions" work now and worry about dupe logs after. Please submit a bug if you come across any. 160576 7 yurys 2009-11-04 12:57:51 -0800 This patch will lead to duplicated messages in console. If there is an exception in the handler it will be first reported to the console because the TryCatch is verbose and then it will be reported in reportException. (In reply to comment #5) > Created an attachment (id=42501) [details] > Proposed Patch 160579 8 yurys 2009-11-04 13:01:10 -0800 Yes, I started to work on the change but we need to prepare for GDD now. I'll send it later. (In reply to comment #2) > I'm not too familiar with how the devtools hook in here. Can you upload a > patch that does the above so we can see what it looks like in code? 160622 9 42526 jianli 2009-11-04 14:58:49 -0800 Created attachment 42526 Proposed Patch Fixed the problem that duplicate error messages could be shown in the console. 160738 10 yurys 2009-11-04 23:06:38 -0800 This patch seems to fix the issue with console duplicates. But please add a FIXME comment and file a bug for this. We should not discriminate between different ScriptExecutionContext types when reporting exceptions. (In reply to comment #9) > Created an attachment (id=42526) [details] > Proposed Patch > > Fixed the problem that duplicate error messages could be shown in the console. 160743 11 42526 pfeldman 2009-11-04 23:09:42 -0800 Comment on attachment 42526 Proposed Patch r+ given FIXME is added and bug is filed. 160872 12 jianli 2009-11-05 10:36:03 -0800 Bug filed: https://bugs.webkit.org/show_bug.cgi?id=31171 FIXME added and committed as http://trac.webkit.org/changeset/50568. 42462 2009-11-04 01:05:33 -0800 2009-11-04 01:05:33 -0800 A test page pause-on-exception.html text/html 222 yurys PGh0bWw+DQo8aGVhZD4NCjxzY3JpcHQ+DQpmdW5jdGlvbiBmKCkgew0KICByZXR1cm4gdW5rbm93 bl92YXI7DQp9DQoNCmZ1bmN0aW9uIGhhbmRsZUNsaWNrKCkgew0KICB2YXIgbyA9IGYoKTsNCn0N Cjwvc2NyaXB0Pg0KPC9oZWFkPg0KPGJvZHk+DQo8aW5wdXQgdHlwZT0nYnV0dG9uJyBvbmNsaWNr PSdoYW5kbGVDbGljaygpJyB2YWx1ZT0nVGVzdCcvPg0KPC9ib2R5Pg0KPC9odG1sPg0K 42501 2009-11-04 11:20:56 -0800 2009-11-04 14:58:49 -0800 Proposed Patch 31108 text/plain 1581 jianli ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NmQ3ZmZhNy4uZmZkNmMzMiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAwOS0xMS0wNCAgSmlhbiBMaSAgPGpp YW5saUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgQnVnIDMxMTA4IC0gW1Y4XSBSRUdSRVNTSU9OOiBQYXVzZSBvbiBleGNlcHRp b24gaXMgYnJva2VuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0zMTEwOAorCisgICAgICAgICogYmluZGluZ3MvdjgvVjhBYnN0cmFjdEV2ZW50TGlzdGVu ZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VjhBYnN0cmFjdEV2ZW50TGlzdGVuZXI6Omludm9r ZUV2ZW50SGFuZGxlcik6CisKIDIwMDktMTEtMDMgIEplcmVteSBPcmxvdyAgPGpvcmxvd0BjaHJv bWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gRmlzaGVyLgpkaWZmIC0tZ2l0 IGEvV2ViQ29yZS9iaW5kaW5ncy92OC9WOEFic3RyYWN0RXZlbnRMaXN0ZW5lci5jcHAgYi9XZWJD b3JlL2JpbmRpbmdzL3Y4L1Y4QWJzdHJhY3RFdmVudExpc3RlbmVyLmNwcAppbmRleCAxYWIyZTA0 Li5mYTQ2MmFkIDEwMDY0NAotLS0gYS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4QWJzdHJhY3RFdmVu dExpc3RlbmVyLmNwcAorKysgYi9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4QWJzdHJhY3RFdmVudExp c3RlbmVyLmNwcApAQCAtMTQxLDcgKzE0MSw2IEBAIHZvaWQgVjhBYnN0cmFjdEV2ZW50TGlzdGVu ZXI6Omludm9rZUV2ZW50SGFuZGxlcihTY3JpcHRFeGVjdXRpb25Db250ZXh0KiBjb250ZXh0CiAg ICAgICAgIHRyeUNhdGNoLlJlc2V0KCk7CiAKICAgICAgICAgLy8gQ2FsbCB0aGUgZXZlbnQgaGFu ZGxlci4KLSAgICAgICAgdHJ5Q2F0Y2guU2V0VmVyYm9zZShmYWxzZSk7IC8vIFdlIGRvIG5vdCB3 YW50IHRvIHJlcG9ydCB0aGUgZXhjZXB0aW9uIHRvIHRoZSBpbnNwZWN0b3IgY29uc29sZS4KICAg ICAgICAgcmV0dXJuVmFsdWUgPSBjYWxsTGlzdGVuZXJGdW5jdGlvbihjb250ZXh0LCBqc0V2ZW50 LCBldmVudCk7CiAgICAgICAgIGlmICghdHJ5Q2F0Y2guQ2FuQ29udGludWUoKSkKICAgICAgICAg ICAgIHJldHVybjsKQEAgLTE1Myw3ICsxNTIsNiBAQCB2b2lkIFY4QWJzdHJhY3RFdmVudExpc3Rl bmVyOjppbnZva2VFdmVudEhhbmRsZXIoU2NyaXB0RXhlY3V0aW9uQ29udGV4dCogY29udGV4dAog ICAgICAgICB9CiAKICAgICAgICAgLy8gUmVzdG9yZSB0aGUgb2xkIGV2ZW50LiBUaGlzIG11c3Qg YmUgZG9uZSBmb3IgYWxsIGV4aXQgcGF0aHMgdGhyb3VnaCB0aGlzIG1ldGhvZC4KLSAgICAgICAg dHJ5Q2F0Y2guU2V0VmVyYm9zZSh0cnVlKTsKICAgICAgICAgaWYgKHNhdmVkRXZlbnQuSXNFbXB0 eSgpKQogICAgICAgICAgICAgdjhDb250ZXh0LT5HbG9iYWwoKS0+U2V0SGlkZGVuVmFsdWUoZXZl bnRTeW1ib2wsIHY4OjpVbmRlZmluZWQoKSk7CiAgICAgICAgIGVsc2UK 42526 2009-11-04 14:58:49 -0800 2009-11-04 23:09:42 -0800 Proposed Patch 31108_2 text/plain 2513 jianli ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NmQ3ZmZhNy4uY2YwMjY4ZiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAwOS0xMS0wNCAgSmlhbiBMaSAgPGpp YW5saUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgQnVnIDMxMTA4IC0gW1Y4XSBSRUdSRVNTSU9OOiBQYXVzZSBvbiBleGNlcHRp b24gaXMgYnJva2VuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0zMTEwOAorCisgICAgICAgICogYmluZGluZ3MvdjgvVjhBYnN0cmFjdEV2ZW50TGlzdGVu ZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VjhBYnN0cmFjdEV2ZW50TGlzdGVuZXI6Omludm9r ZUV2ZW50SGFuZGxlcik6CisgICAgICAgICogYmluZGluZ3MvdjgvVjhVdGlsaXRpZXMuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6cmVwb3J0RXhjZXB0aW9uKToKKwogMjAwOS0xMS0wMyAgSmVyZW15 IE9ybG93ICA8am9ybG93QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXJp biBGaXNoZXIuCmRpZmYgLS1naXQgYS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4QWJzdHJhY3RFdmVu dExpc3RlbmVyLmNwcCBiL1dlYkNvcmUvYmluZGluZ3MvdjgvVjhBYnN0cmFjdEV2ZW50TGlzdGVu ZXIuY3BwCmluZGV4IDFhYjJlMDQuLmZhNDYyYWQgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvYmluZGlu Z3MvdjgvVjhBYnN0cmFjdEV2ZW50TGlzdGVuZXIuY3BwCisrKyBiL1dlYkNvcmUvYmluZGluZ3Mv djgvVjhBYnN0cmFjdEV2ZW50TGlzdGVuZXIuY3BwCkBAIC0xNDEsNyArMTQxLDYgQEAgdm9pZCBW OEFic3RyYWN0RXZlbnRMaXN0ZW5lcjo6aW52b2tlRXZlbnRIYW5kbGVyKFNjcmlwdEV4ZWN1dGlv bkNvbnRleHQqIGNvbnRleHQKICAgICAgICAgdHJ5Q2F0Y2guUmVzZXQoKTsKIAogICAgICAgICAv LyBDYWxsIHRoZSBldmVudCBoYW5kbGVyLgotICAgICAgICB0cnlDYXRjaC5TZXRWZXJib3NlKGZh bHNlKTsgLy8gV2UgZG8gbm90IHdhbnQgdG8gcmVwb3J0IHRoZSBleGNlcHRpb24gdG8gdGhlIGlu c3BlY3RvciBjb25zb2xlLgogICAgICAgICByZXR1cm5WYWx1ZSA9IGNhbGxMaXN0ZW5lckZ1bmN0 aW9uKGNvbnRleHQsIGpzRXZlbnQsIGV2ZW50KTsKICAgICAgICAgaWYgKCF0cnlDYXRjaC5DYW5D b250aW51ZSgpKQogICAgICAgICAgICAgcmV0dXJuOwpAQCAtMTUzLDcgKzE1Miw2IEBAIHZvaWQg VjhBYnN0cmFjdEV2ZW50TGlzdGVuZXI6Omludm9rZUV2ZW50SGFuZGxlcihTY3JpcHRFeGVjdXRp b25Db250ZXh0KiBjb250ZXh0CiAgICAgICAgIH0KIAogICAgICAgICAvLyBSZXN0b3JlIHRoZSBv bGQgZXZlbnQuIFRoaXMgbXVzdCBiZSBkb25lIGZvciBhbGwgZXhpdCBwYXRocyB0aHJvdWdoIHRo aXMgbWV0aG9kLgotICAgICAgICB0cnlDYXRjaC5TZXRWZXJib3NlKHRydWUpOwogICAgICAgICBp ZiAoc2F2ZWRFdmVudC5Jc0VtcHR5KCkpCiAgICAgICAgICAgICB2OENvbnRleHQtPkdsb2JhbCgp LT5TZXRIaWRkZW5WYWx1ZShldmVudFN5bWJvbCwgdjg6OlVuZGVmaW5lZCgpKTsKICAgICAgICAg ZWxzZQpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9iaW5kaW5ncy92OC9WOFV0aWxpdGllcy5jcHAgYi9X ZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4VXRpbGl0aWVzLmNwcAppbmRleCBlY2FjMzU4Li40OTg5Yjdk IDEwMDY0NAotLS0gYS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4VXRpbGl0aWVzLmNwcAorKysgYi9X ZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4VXRpbGl0aWVzLmNwcApAQCAtMTQ3LDcgKzE0NywxMCBAQCB2 b2lkIHJlcG9ydEV4Y2VwdGlvbihTY3JpcHRTdGF0ZSogc2NyaXB0U3RhdGUsIHY4OjpUcnlDYXRj aCYgZXhjZXB0aW9uQ2F0Y2hlcikKICAgICAgICAgc291cmNlVVJMID0gdG9XZWJDb3JlU3RyaW5n KG1lc3NhZ2UtPkdldFNjcmlwdFJlc291cmNlTmFtZSgpKTsKICAgICB9CiAKLSAgICBnZXRTY3Jp cHRFeGVjdXRpb25Db250ZXh0KHNjcmlwdFN0YXRlKS0+cmVwb3J0RXhjZXB0aW9uKGVycm9yTWVz c2FnZSwgbGluZU51bWJlciwgc291cmNlVVJMKTsKKyAgICAvLyBEbyBub3QgcmVwb3J0IHRoZSBl eGNlcHRpb24gaWYgdGhlIGN1cnJlbnQgZXhlY3V0aW9uIGNvbnRleHQgaXMgRG9jdW1lbnQgYmVj YXVzZSB3ZSBkbyBub3Qgd2FudCB0byBsZWFkIHRvIGR1cGxpY2F0ZSBlcnJvciBtZXNzYWdlcyBp biB0aGUgY29uc29sZS4KKyAgICBTY3JpcHRFeGVjdXRpb25Db250ZXh0KiBjb250ZXh0ID0gZ2V0 U2NyaXB0RXhlY3V0aW9uQ29udGV4dChzY3JpcHRTdGF0ZSk7CisgICAgaWYgKCFjb250ZXh0LT5p c0RvY3VtZW50KCkpCisgICAgICBjb250ZXh0LT5yZXBvcnRFeGNlcHRpb24oZXJyb3JNZXNzYWdl LCBsaW5lTnVtYmVyLCBzb3VyY2VVUkwpOwogICAgIGV4Y2VwdGlvbkNhdGNoZXIuUmVzZXQoKTsK IH0KIAo=