RESOLVED FIXED 41958
[Chromium] Crash when stepping on a breakpoint while debugging Web Inspector 
https://bugs.webkit.org/show_bug.cgi?id=41958
Summary [Chromium] Crash when stepping on a breakpoint while debugging Web Inspector
Alexander Pavlov (apavlov)
Reported 2010-07-09 07:19:45 PDT
An empty page group gets in the way in this case, which results into a crash.
Attachments
[PATCH] Suggested solution (3.15 KB, patch)
2010-07-09 07:46 PDT, Alexander Pavlov (apavlov) 		no flags
DetailsFormatted DiffDiff
[PATCH] Comments addressed (4.46 KB, patch)
2010-07-12 05:47 PDT, Alexander Pavlov (apavlov) 		yurys: review-
DetailsFormatted DiffDiff
[PATCH] Use Vector instead of std::stack (4.56 KB, patch)
2010-07-12 06:30 PDT, Alexander Pavlov (apavlov) 		yurys: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexander Pavlov (apavlov)
Comment 1 2010-07-09 07:46:02 PDT
Created attachment 61045 [details] [PATCH] Suggested solution
Yury Semikhatsky
Comment 2 2010-07-09 08:01:30 PDT
Comment on attachment 61045 [details] [PATCH] Suggested solution WebKit/chromium/src/WebViewImpl.cpp:193 + pageGroupLoadDeferrer = new PageGroupLoadDeferrer(pageGroupLoadDeferrer); I'd rather use a regular stack of PageGroupLoadDeferrers and push 0 when page group is empty.
Alexander Pavlov (apavlov)
Comment 3 2010-07-12 05:47:05 PDT
Created attachment 61213 [details] [PATCH] Comments addressed
Yury Semikhatsky
Comment 4 2010-07-12 06:09:27 PDT
Comment on attachment 61213 [details] [PATCH] Comments addressed WebKit/chromium/src/WebViewImpl.cpp:144 + static std::stack<PageGroupLoadDeferrer*> pageGroupLoadDeferrerStack; "WTF container classes should be used in the implementation of this[chromium/src] API."
Alexander Pavlov (apavlov)
Comment 5 2010-07-12 06:30:51 PDT
Created attachment 61220 [details] [PATCH] Use Vector instead of std::stack
Yury Semikhatsky
Comment 6 2010-07-12 06:34:19 PDT
Comment on attachment 61220 [details] [PATCH] Use Vector instead of std::stack WebKit/chromium/src/WebViewImpl.cpp:194 + pageGroupLoadDeferrerStack.append(static_cast<PageGroupLoadDeferrer*>(0)); This cast is weird. WebKit/chromium/src/WebViewImpl.cpp:206 + if (pageGroupLoadDeferrer) You don't need this check.
Alexander Pavlov (apavlov)
Comment 7 2010-07-12 07:07:42 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/page/PageGroupLoadDeferrer.cpp M WebCore/page/PageGroupLoadDeferrer.h M WebKit/chromium/ChangeLog M WebKit/chromium/src/WebViewImpl.cpp Committed r63080
Alexander Pavlov (apavlov)
Comment 8 2010-07-12 07:09:28 PDT
(In reply to comment #6) > (From update of attachment 61220 [details]) > WebKit/chromium/src/WebViewImpl.cpp:194 > + pageGroupLoadDeferrerStack.append(static_cast<PageGroupLoadDeferrer*>(0)); > This cast is weird. Otherwise I get a compile error. > WebKit/chromium/src/WebViewImpl.cpp:206 > + if (pageGroupLoadDeferrer) > You don't need this check. Fixed.
Note You need to log in before you can comment on or make changes to this bug.