Bug 41958

Summary:

[Chromium] Crash when stepping on a breakpoint while debugging Web Inspector

Product:

WebKit

Reporter:

Alexander Pavlov (apavlov) <apavlov>

Component:

WebKit Misc.

Assignee:

Alexander Pavlov (apavlov) <apavlov>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

pfeldman, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
[PATCH] Suggested solution	none
[PATCH] Comments addressed	yurys: review-
[PATCH] Use Vector instead of std::stack	yurys: review+

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	Details Formatted Diff Diff
[PATCH] Comments addressed (4.46 KB, patch) 2010-07-12 05:47 PDT, Alexander Pavlov (apavlov)	yurys: review-	Details Formatted Diff Diff
[PATCH] Use Vector instead of std::stack (4.56 KB, patch) 2010-07-12 06:30 PDT, Alexander Pavlov (apavlov)	yurys: review+	Details Formatted Diff Diff
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.