WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
119389
Make WebHistory more type safe
https://bugs.webkit.org/show_bug.cgi?id=119389
Summary
Make WebHistory more type safe
Patrick R. Gansterer
Reported
2013-08-01 04:38:43 PDT
Make WebHistory more type safe
Attachments
Patch
(11.48 KB, patch)
2013-08-01 04:45 PDT
,
Patrick R. Gansterer
no flags
Details
Formatted Diff
Diff
Patch
(11.55 KB, patch)
2013-09-15 18:29 PDT
,
Patrick R. Gansterer
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Patrick R. Gansterer
Comment 1
2013-08-01 04:45:28 PDT
Created
attachment 207912
[details]
Patch
WebKit Commit Bot
Comment 2
2013-08-01 04:47:10 PDT
Attachment 207912
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/WebHistory.cpp', u'Source/WebKit/win/WebHistory.h']" exit_code: 1 Source/WebKit/win/WebHistory.cpp:532: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/win/WebHistory.cpp:533: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brent Fulgham
Comment 3
2013-09-15 17:37:50 PDT
Comment on
attachment 207912
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=207912&action=review
Looks good! Can you please use 'auto'?
> Source/WebKit/win/WebHistory.cpp:246 > + for (URLToEntriesMap::const_iterator it = m_entriesByURL.begin(); it != m_entriesByURL.end(); ++it)
Can this be for (auto it = ...)?
> Source/WebKit/win/WebHistory.cpp:345 > + for (URLToEntriesMap::const_iterator it = m_entriesByURL.begin(); it != m_entriesByURL.end(); ++i, ++it) {
auto!
>> Source/WebKit/win/WebHistory.cpp:532 >> + /* [in] */ BSTR url, > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
I've been changing these to be one-line in "WebKit Style" when I'm changing the signature. Do we really need the STDMETHODCALLTYPE here? I thought that if it was in the header file that was sufficient.
> Source/WebKit/win/WebHistory.cpp:539 > + URLToEntriesMap::iterator it = m_entriesByURL.find(url);
auto it = ...
> Source/WebKit/win/WebHistory.cpp:549 > + URLToEntriesMap::iterator it = m_entriesByURL.find(urlString);
auto ...
> Source/WebKit/win/WebHistory.cpp:757 > + for (URLToEntriesMap::const_iterator it = m_entriesByURL.begin(); it != m_entriesByURL.end(); ++it) {
auto...
Patrick R. Gansterer
Comment 4
2013-09-15 18:29:44 PDT
Created
attachment 211733
[details]
Patch
Brent Fulgham
Comment 5
2013-09-15 19:41:29 PDT
Comment on
attachment 211733
[details]
Patch r=me
WebKit Commit Bot
Comment 6
2013-09-18 10:31:26 PDT
Comment on
attachment 211733
[details]
Patch Clearing flags on attachment: 211733 Committed
r156051
: <
http://trac.webkit.org/changeset/156051
>
WebKit Commit Bot
Comment 7
2013-09-18 10:31:27 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