RESOLVED FIXED Bug 41477
Direct calls to cast operator in WebFrameImpl.cpp
https://bugs.webkit.org/show_bug.cgi?id=41477
Summary Direct calls to cast operator in WebFrameImpl.cpp
David Holloway
Reported 2010-07-01 11:57:56 PDT
Overview: Poor C++ style with direct calls to cast operator in WebFrameImpl.cpp. Steps to Reproduce: 1) Open WebFrameImpl.cpp 2) Inspect WebKit::WebFrameImpl::registerPasswordListener and WebKit::WebFrameImpl::notifiyPasswordListenerOfAutocomplete Actual Results: Notice the calls to: RefPtr<HTMLInputElement> element = inputElement.operator PassRefPtr<HTMLInputElement>(); This is using an explicit call to a PassRefPtr cast operator to bridge from a WebInputElement back to an HTMLInputElement. Expected Results: Better style is to "unwrap" the WebInputElement explicitly, eg.: RefPtr<HTMLInputElement> element(inputElement.unwrap<HTMLInputElement>());
Attachments
Proposed patch. (2.60 KB, patch)
2010-07-01 12:34 PDT, David Holloway
no flags
Proposed patch. (2.61 KB, patch)
2010-07-01 13:09 PDT, David Holloway
no flags
Proposed patch. Fixes const-ness issues. (4.01 KB, patch)
2010-07-07 15:46 PDT, David Holloway
no flags
David Holloway
Comment 1 2010-07-01 12:31:43 PDT
Follow up for bug 41236.
David Holloway
Comment 2 2010-07-01 12:34:15 PDT
Created attachment 60272 [details] Proposed patch.
David Holloway
Comment 3 2010-07-01 13:09:17 PDT
Created attachment 60277 [details] Proposed patch.
Darin Fisher (:fishd, Google)
Comment 4 2010-07-01 13:18:13 PDT
Comment on attachment 60277 [details] Proposed patch. Thanks! R=me
WebKit Commit Bot
Comment 5 2010-07-01 19:50:48 PDT
Comment on attachment 60277 [details] Proposed patch. Clearing flags on attachment: 60277 Committed r62349: <http://trac.webkit.org/changeset/62349>
WebKit Commit Bot
Comment 6 2010-07-01 19:50:52 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 7 2010-07-01 20:00:37 PDT
http://trac.webkit.org/changeset/62349 might have broken Chromium Linux Release
David Levin
Comment 8 2010-07-01 20:15:11 PDT
Rolled out due to breakage on Linux. http://trac.webkit.org/changeset/62351
David Levin
Comment 9 2010-07-01 20:17:45 PDT
And OSX bustage. Here's the compile error: /Users/cltbld/Desktop/BuildSlaveData/WebKit-BuildSlave/chromium-mac-release/build/WebKit/chromium/src/WebFrameImpl.cpp: In member function ‘virtual void WebKit::WebFrameImpl::notifiyPasswordListenerOfAutocomplete(const WebKit::WebInputElement&)’: /Users/cltbld/Desktop/BuildSlaveData/WebKit-BuildSlave/chromium-mac-release/build/WebKit/chromium/src/WebFrameImpl.cpp:1974: error: passing ‘const WebKit::WebInputElement’ as ‘this’ argument of ‘T* WebKit::WebNode::unwrap() [with T = WebCore::HTMLInputElement]’ discards qualifiers
David Holloway
Comment 10 2010-07-07 15:46:47 PDT
Created attachment 60792 [details] Proposed patch. Fixes const-ness issues. Grrrr. False positive my end. New patch addresses const-ness issues and pushes the const-ness of the |inputElement| parameter down a level to WebFrameImpl::getPasswordListener(). We need a const_cast at this level due to the use of non-const RefPtr<HTMLInputElement> used as the key to the |m_passwordListeners| map.
WebKit Commit Bot
Comment 11 2010-07-08 01:49:15 PDT
Comment on attachment 60792 [details] Proposed patch. Fixes const-ness issues. Clearing flags on attachment: 60792 Committed r62770: <http://trac.webkit.org/changeset/62770>
WebKit Commit Bot
Comment 12 2010-07-08 01:49:20 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.