[webkit-reviews] review granted: [Bug 221226] Add scaffolding to request image extraction after a delay when hovering over images : [Attachment 418930] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 221226: Add scaffolding to request image extraction after a delay when
hovering over images
https://bugs.webkit.org/show_bug.cgi?id=221226

Attachment 418930: Patch

https://bugs.webkit.org/attachment.cgi?id=418930&action=review




--- Comment #4 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 418930
  --> https://bugs.webkit.org/attachment.cgi?id=418930
Patch

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

r=me

> Source/WebCore/page/EventHandler.cpp:1884
> +    if (m_imageExtractionTimer.isActive())
> +	   m_imageExtractionTimer.restart();

As we discussed offline, we should probably not start the timer for synthetic
mouse events (such as on iOS).

> Source/WebCore/page/EventHandler.cpp:2515
> +    if (targetElement && is<RenderImage>(targetElement->renderer()))

NIT: i usually prefer to use member variables instead of parameters once the
parameter is assigned to the member variable.

>>> Source/WebKit/WebProcess/WebPage/WebPage.cpp:7141
>>> +	 m_elementsWithExtractedImages.add(element);
>> 
>> When are items removed (other than page navigation)?  What would happen if I
hover over the same element twice?
> 
> This is answered in the ChangeLog (essentially, we won't attempt to
re-request image extraction when hovering over an element again).

My apologies I totally missed that ��