Bug 122230 – Reduce duplicated code in WebPageProxy

• Component: New Bugs
• Status: RESOLVED FIXED

[Gustavo Noronha (kov)]

Reduce duplicated code in WebPageProxy

[Gustavo Noronha (kov)]

Created attachment 213188 [details]
Patch

[Darin Adler]

Comment on attachment 213188 [details]
Patch

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?

[Gustavo Noronha (kov)]

Created attachment 213247 [details]
Patch

Thanks for the review! I'm not too fond of the resetState name myself, but couldn't think of anything better. All of your other comments were mistakes on my part, all fixed with this update.

[Gustavo Noronha (kov)]

Comment on attachment 213247 [details]
Patch

Clearing flags on attachment: 213247

Committed r157064: <http://trac.webkit.org/changeset/157064>

[Gustavo Noronha (kov)]

All reviewed patches have been landed.  Closing bug.