WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(7.01 KB, patch)
2013-10-03 08:17 PDT
,
Gustavo Noronha (kov)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Gustavo Noronha (kov)
Comment 1
2013-10-02 13:09:40 PDT
Created
attachment 213188
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug