[webkit-reviews] review denied: [Bug 122230] Reduce duplicated code in WebPageProxy : [Attachment 213188] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 122230: Reduce duplicated code in WebPageProxy
https://bugs.webkit.org/show_bug.cgi?id=122230

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213188&action=review


Patch seems like a good idea, but there is at least one small error, and a few
questions.

Not sure that “reset state” is a great name for this new function, kind of
vague, but I can’t quickly think of a better one.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:-555
> -    if (m_inspector) {

The code in resetState does not do this null check. What guarantees this won’t
be null?

> Source/WebKit2/UIProcess/WebPageProxy.cpp:-562
> -    if (m_fullScreenManager) {

The code in resetState does not do this null check. What guarantees this won’t
be null?

> Source/WebKit2/UIProcess/WebPageProxy.cpp:-605
> -    invalidateCallbackMap(m_imageCallbacks);

This line of code was lost. I don’t see it in resetState.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:-618
> -    Vector<WebEditCommandProxy*> editCommandVector;
> -    copyToVector(m_editCommandSet, editCommandVector);
> -    m_editCommandSet.clear();
> -    for (size_t i = 0, size = editCommandVector.size(); i < size; ++i)
> -	   editCommandVector[i]->invalidate();

The resetState function calls m_pageClient->clearAllEditCommands(), but this
code did not. Why is that change in behavior OK?