RESOLVED FIXED 122230
Reduce duplicated code in WebPageProxy
https://bugs.webkit.org/show_bug.cgi?id=122230
Summary Reduce duplicated code in WebPageProxy
Gustavo Noronha (kov)
Reported 2013-10-02 13:07:30 PDT
Reduce duplicated code in WebPageProxy
Attachments
Patch (5.87 KB, patch)
2013-10-02 13:09 PDT, Gustavo Noronha (kov)
no flags
Patch (7.01 KB, patch)
2013-10-03 08:17 PDT, Gustavo Noronha (kov)
no flags
Gustavo Noronha (kov)
Comment 1 2013-10-02 13:09:40 PDT
Darin Adler
Comment 2 2013-10-02 20:53:56 PDT
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)
Comment 3 2013-10-03 08:17:53 PDT
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 4 2013-10-07 16:00:07 PDT
Comment on attachment 213247 [details] Patch Clearing flags on attachment: 213247 Committed r157064: <http://trac.webkit.org/changeset/157064>
Gustavo Noronha (kov)
Comment 5 2013-10-07 16:00:13 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.