216385 2020-09-10 16:03:54 -0700 Web Inspector: Change `InspectorAnimationAgent->startTracking` to not error on repeated calls 2020-09-22 21:47:47 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Local Build All All RESOLVED FIXED InRadar P2 Normal --- 1 bburg pangle bburg hi inspector-bugzilla-changes joepeck pangle webkit-bug-importer oldest_to_newest 1687582 0 bburg 2020-09-10 16:03:54 -0700 STEPS TO REPRODUCE: - navigate to brrian.org - open inspector to Timelines tab, with Media & Animations timeline enabled - navigate to nfl.com - Uncaught exception "Animation domain already tracking (at Connection.js:​162:​29)" seems like animation instrumentation is auto-starting, or has bad initial state upon cross-domain navigation. I wonder if it happens on a same-domain or same-document navigation? 1687585 1 webkit-bug-importer 2020-09-10 16:15:25 -0700 <rdar://problem/68670991> 1690329 2 pangle 2020-09-21 09:48:47 -0700 I'm unable to reproduce this issue on same-domain navigation. It seems to be limited to cross-domain navigation. 1690488 3 joepeck 2020-09-21 13:30:48 -0700 This seems likely to be harmless. Whenever the Timeline Tab is in the foreground the frontend notifies the backend of the open set of timelines so that the backend can automatically start an appropriate timeline recording on page navigations (`Timeline.setAutoCaptureEnabled` and `Timeline.setInstruments`). What might happen on cross domain navigation is that both the backend and frontend try to start a recording? It seems the Animation domain errors if startTracking is called multiple times, but most other timelines don't (Memory, Heap, CPUProfiler, ScriptProfiler, etc.). 1690490 4 hi 2020-09-21 13:39:57 -0700 (In reply to Joseph Pecoraro from comment #3) > This seems likely to be harmless. To expand on this further, whenever you see the uncaught exception UI in Web Inspector in an engineering build, it can mean one of two things: 1) an exception in JavaScript was thrown 2) some protocol command sent an error payload/message back to the frontend In the case of (1), that's definitely a bug and should be fixed. In the case of (2), most of the commands we invoke do not have error handling cases because we don't ever expect them to error. Most of the time the error is harmless and will not affect the user in any negative way (usually if it does affect the user then we'd see an actual JS error like (1) right after the (2) error). A simple way to tell (most of the time) is to just hit the "close" link to dismiss the uncaught exception UI and try to continue using Web Inspector as normal (if nothing is wrong, then the error is likely harmless) > What might happen on cross domain navigation is that both the backend and frontend try to start a recording? It seems the Animation domain errors if startTracking is called multiple times, but most other timelines don't (Memory, Heap, CPUProfiler, ScriptProfiler, etc.). Yeah I think we should just make `Animation.startTracking` not error. 1690516 5 pangle 2020-09-21 14:21:53 -0700 (In reply to Devin Rousso from comment #4) > (In reply to Joseph Pecoraro from comment #3) > > This seems likely to be harmless. > To expand on this further, whenever you see the uncaught exception UI in Web > Inspector in an engineering build, it can mean one of two things: > 1) an exception in JavaScript was thrown > 2) some protocol command sent an error payload/message back to the frontend > In the case of (1), that's definitely a bug and should be fixed. > In the case of (2), most of the commands we invoke do not have error > handling cases because we don't ever expect them to error. Most of the time > the error is harmless and will not affect the user in any negative way > (usually if it does affect the user then we'd see an actual JS error like > (1) right after the (2) error). A simple way to tell (most of the time) is > to just hit the "close" link to dismiss the uncaught exception UI and try to > continue using Web Inspector as normal (if nothing is wrong, then the error > is likely harmless) I understand that most of these are harmless, but the noise makes it difficult to identify what is actually a bug and what is "expected" behavior. > > What might happen on cross domain navigation is that both the backend and frontend try to start a recording? It seems the Animation domain errors if startTracking is called multiple times, but most other timelines don't (Memory, Heap, CPUProfiler, ScriptProfiler, etc.). > Yeah I think we should just make `Animation.startTracking` not error. This is certainly a relatively quick fix then. 1690699 6 409360 pangle 2020-09-22 08:21:23 -0700 Created attachment 409360 Patch 1690783 7 409360 hi 2020-09-22 11:17:05 -0700 Comment on attachment 409360 Patch r=me :) 1691028 8 ews-feeder 2020-09-22 21:47:46 -0700 Committed r267464: <https://trac.webkit.org/changeset/267464> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409360. 409360 2020-09-22 08:21:23 -0700 2020-09-22 21:47:47 -0700 Patch bug-216385-20200922112123.patch text/plain 1671 pangle U3VidmVyc2lvbiBSZXZpc2lvbjogMjY3MzQ0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDkzMzU1ODMyODQxOTk5 ZjY1MjAwZGQ0ODQ5MThlMzc3ODhiN2NmYS4uY2ZmMDVlOTdmMzAyM2YyYThlZThmNzFiNzYyMjZj MThkMDI3Njk1ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDIwLTA5LTIyICBQYXRy aWNrIEFuZ2xlICA8cGFuZ2xlQGFwcGxlLmNvbT4KKworICAgICAgICBXZWIgSW5zcGVjdG9yOiBD aGFuZ2UgYEluc3BlY3RvckFuaW1hdGlvbkFnZW50LT5zdGFydFRyYWNraW5nYCB0byBub3QgZXJy b3Igb24gcmVwZWF0ZWQgY2FsbHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTIxNjM4NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIElucHNlY3RvckFuaW1hdGlvbkFnZW50IG5vdyBtaXJyb3JzIHRoZSBiZWhh dmlvciBvZiBvdGhlciB0aW1lbGluZSBhZ2VudHMgYnkgbm8gbG9uZ2VyIHJldHVybmluZyBhbiBl cnJvciBvbiBhCisgICAgICAgIHJlcGVhdGVkIGludm9jYXRpb24gb2YgYHN0YXJ0VHJhY2tpbmdg LgorCisgICAgICAgICogaW5zcGVjdG9yL2FnZW50cy9JbnNwZWN0b3JBbmltYXRpb25BZ2VudC5j cHA6CisgICAgICAgIChXZWJDb3JlOjpJbnNwZWN0b3JBbmltYXRpb25BZ2VudDo6c3RhcnRUcmFj a2luZyk6CisKIDIwMjAtMDktMjEgIFphbGFuIEJ1anRhcyAgPHphbGFuQGFwcGxlLmNvbT4KIAog ICAgICAgICBbTEZDXVtCRkNdIE1vdmUgaGFzQ2xlYXJhbmNlIG91dCBvZiBCb3hHZW9tZXRyeQpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL2FnZW50cy9JbnNwZWN0b3JBbmlt YXRpb25BZ2VudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvYWdlbnRzL0luc3BlY3Rv ckFuaW1hdGlvbkFnZW50LmNwcAppbmRleCAwMjU3MTJiMDk0NjRlYzZiMTU0N2VjZjcwNjZlMjBi ZGZhYmY0MWNmLi5kZjNlZjVlNWFhYmE3YjNkM2QyY2Q4YWEyYjMzYmM2OTU0MDYyYmQxIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvYWdlbnRzL0luc3BlY3RvckFuaW1hdGlv bkFnZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvYWdlbnRzL0luc3BlY3Rv ckFuaW1hdGlvbkFnZW50LmNwcApAQCAtMzQwLDcgKzM0MCw3IEBAIFByb3RvY29sOjpFcnJvclN0 cmluZ09yPFJlZjxQcm90b2NvbDo6UnVudGltZTo6UmVtb3RlT2JqZWN0Pj4gSW5zcGVjdG9yQW5p bWF0aW9uCiBQcm90b2NvbDo6RXJyb3JTdHJpbmdPcjx2b2lkPiBJbnNwZWN0b3JBbmltYXRpb25B Z2VudDo6c3RhcnRUcmFja2luZygpCiB7CiAgICAgaWYgKG1faW5zdHJ1bWVudGluZ0FnZW50cy50 cmFja2luZ0FuaW1hdGlvbkFnZW50KCkgPT0gdGhpcykKLSAgICAgICAgcmV0dXJuIG1ha2VVbmV4 cGVjdGVkKCJBbmltYXRpb24gZG9tYWluIGFscmVhZHkgdHJhY2tpbmciX3MpOworICAgICAgICBy ZXR1cm4geyB9OwogCiAgICAgbV9pbnN0cnVtZW50aW5nQWdlbnRzLnNldFRyYWNraW5nQW5pbWF0 aW9uQWdlbnQodGhpcyk7CiAK