[webkit-reviews] review granted: [Bug 131319] Allow elements to register for changes in page scale : [Attachment 228773] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Dean Jackson
<dino at apple.com>'s request for review:
Bug 131319: Allow elements to register for changes in page scale
https://bugs.webkit.org/show_bug.cgi?id=131319

Attachment 228773: Patch
https://bugs.webkit.org/attachment.cgi?id=228773&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=228773&action=review


> Source/WebCore/ChangeLog:25
> +	   * html/HTMLMediaElement.cpp:
> +	   (WebCore::HTMLMediaElement::HTMLMediaElement):
> +	   (WebCore::HTMLMediaElement::~HTMLMediaElement): Remove ourselves
from the
> +	   callback if necessary.

Nit: you forgot "setMediaControlsDependOnPageScaleFactor".

> Source/WebCore/ChangeLog:28
> +	   (WebCore::HTMLMediaElement::mediaControlsDependOnPageScaleFactor):
Add ourselves to

Nit: This is the accessor.

> Source/WebCore/ChangeLog:31
> +	   (WebCore::Page::setPageScaleFactor): Tell the document that the user
has zoomed.

Nit: "the document" ->	"all documents"

> Source/WebCore/dom/Document.cpp:4199
> +void Document::registerForPageScaleFactorChangedCallbacks(HTMLMediaElement*
e)

Nit: might as well spell out "element".

> Source/WebCore/dom/Document.cpp:4204
> +void
Document::unregisterForPageScaleFactorChangedCallbacks(HTMLMediaElement* e)

Ditto.

> Source/WebCore/html/HTMLMediaElement.cpp:422
> +    if (m_mediaControlsDependOnPageScaleFactor)
> +	   document().unregisterForPageScaleFactorChangedCallbacks(this);

Can this go in HTMLMediaElement::unregisterWithDocument?

> Source/WebCore/html/HTMLMediaElement.cpp:5986
> +void HTMLMediaElement::setMediaControlsDependOnPageScaleFactor(bool v)

Nit: you might as well use a descriptive variable name.

> Source/WebCore/html/HTMLMediaElement.h:892
> +    bool m_mediaControlsDependOnPageScaleFactor : 1;

Nit: this should be grouped with the other 1-bit bools to make the element
slightly smaller.