[webkit-reviews] review granted: [Bug 106146] Show label automatically for plugins of significant size : [Attachment 181421] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jan 4 19:38:51 PST 2013
Simon Fraser (smfr) <simon.fraser at apple.com> has granted Jon Lee
<jonlee at apple.com>'s request for review:
Bug 106146: Show label automatically for plugins of significant size
https://bugs.webkit.org/show_bug.cgi?id=106146
Attachment 181421: Patch
https://bugs.webkit.org/attachment.cgi?id=181421&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=181421&action=review
> Source/WebCore/html/HTMLPlugInImageElement.cpp:293
> + LayoutRect plugInClipRect =
node->renderer()->absoluteClippedOverflowRect();
Is node->renderer() always non-null?
> Source/WebCore/html/HTMLPlugInImageElement.cpp:352
> + if
(shouldPlugInShowLabelAutomatically(document()->page()->mainFrame()->view()->co
ntentsSize(), this))
Is everything in this chain always non-null?
> Source/WebCore/platform/Timer.h:143
> + void setDelay(double delay) { m_delay = delay; }
Not sure you need this (see later comment).
> Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:276
> + m_showLabelDelayTimer.setDelay(reason == UserMousedOver ?
showLabelAfterMouseOverDelay : showLabelAutomaticallyDelay);
> + m_showLabelDelayTimer.restart();
Can't you just call startOneShot() and avoid touching Timer.h? I don't think
you even need to call stop() beforehand.
More information about the webkit-reviews
mailing list