[Webkit-unassigned] [Bug 213306] WIP: we should be able to capture mouse, keyboard, and wheel events to investigate tracking

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 17 13:34:38 PDT 2020


https://bugs.webkit.org/show_bug.cgi?id=213306

--- Comment #4 from katherine_cheney at apple.com ---
Comment on attachment 402138
  --> https://bugs.webkit.org/attachment.cgi?id=402138
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402138&action=review

Yay first patch!! Excited to see work being done in this space.

> Source/WebCore/page/EventCollector.cpp:40
> +    WTFLogAlways("Mouse event on page %llu at x=%d, y=%d, type %d, button %d, clickcount %d, force %f", pageID.toUInt64(), platformMouseEvent.position().x(), platformMouseEvent.position().y(), platformMouseEvent.type(), platformMouseEvent.button(), platformMouseEvent.clickCount(), platformMouseEvent.force());

Wondering if this should instead be RELEASE_LOG_IF_ALLOWED? Or maybe even only logged in debug builds. See CachedResource.cpp for examples.

> Source/WebCore/page/EventCollector.cpp:50
> +    WTFLogAlways("Wheel event at x=%d, y=%d, ticksX %f, ticksY %f, granularity %d, velocity width %f height %f", platformWheelEvent.position().x(), platformWheelEvent.position().y(), platformWheelEvent.wheelTicksX(), platformWheelEvent.wheelTicksY(), platformWheelEvent.granularity(), platformWheelEvent.scrollingVelocity().width(), platformWheelEvent.scrollingVelocity().height());

Ditto.

> Source/WebCore/page/EventCollector.cpp:55
> +    WTFLogAlways("Page %llu load complete", webPageID.toUInt64());

Ditto.

> Source/WebCore/page/EventCollector.h:33
> +

I think no space here.

>> Source/WebCore/page/EventCollector.h:63
>> +    // something like this but with hashable key
> 
> If WallTime is not hashable you can try getting its raw seconds.

You could use a double here? secondsSinceEpoch().value() would convert a WallTime value into a double.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:114
>> +    , m_eventCollector(EventCollector::create())
> 
> I'm not sure this does what you want it to do. You're creating two different EventCollector objects – one in the network process and one in every web content process. So when you call it here in the network process to say that something happened, it's not going to be the same object the ones in the web content processes. My impression is that such an object is only needed in the web content processes. Is the instance here in the network process only for the purposes of telling the web content process for a specific page ID that the page loaded? If so, I'm sure there is such an event already available in the web content process since the page code itself gets the onload event.

I agree EventCollector should probably just be used in the web content process. You could maybe create an EventCollector for each WebPage object, and send collected data to the NetworkProcess in batches when the WebPage closes or reloads.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200617/7ce95598/attachment-0001.htm>


More information about the webkit-unassigned mailing list