Bug 41697

Summary: [Chromium] Crash when re-entering message loop
Product: WebKit Reporter: Alexander Pavlov (apavlov) <apavlov>
Component: WebKit Misc.Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: fishd, masterov, pfeldman, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Suggested solution
yurys: review-
[PATCH] Comments addressed
none
[PATCH] Implemented a stack of PageGroupLoadDeferrers, as suggested by fishd fishd: review+

Description Alexander Pavlov (apavlov) 2010-07-06 07:20:21 PDT
ASSERT(!pageGroupLoadDeferrer) fails in void WebView::willEnterModalLoop() when re-entering message loop (e.g. when evaluating properties that require message loop to be run (e.g. document.cookie) in Web Inspector, on a breakpoint.)
Comment 1 Alexander Pavlov (apavlov) 2010-07-06 08:20:53 PDT
Created attachment 60633 [details]
[PATCH] Suggested solution

Enable re-entrancy of the message loop by tracking the nesting level.
Comment 2 Yury Semikhatsky 2010-07-07 02:33:14 PDT
Comment on attachment 60633 [details]
[PATCH] Suggested solution

WebKit/chromium/src/WebViewImpl.cpp:141
 +  static PageGroupLoadDeferrer* pageGroupLoadDeferrer = 0;
You don't need explicit initialization here.

WebKit/chromium/ChangeLog:5
 +          [Chromium] Crash when re-entering message loop
Please provide more detailed comment including the description of the drawbacks of this solution.

WebKit/chromium/src/WebViewImpl.cpp:191
 +          ASSERT(pageGroupLoadDeferrer);
You may want to check in debug mode that PageGroup hasn't changed since previous call to this method

WebKit/chromium/src/js/Tests.js:1862
 +      WebInspector.console.visible = true;
Use  WebInspector.showConsole instead.

WebKit/chromium/src/js/Tests.js:1896
 +          function(callFrames) {
Please use evaluateInConsole_ instead of this custom code.

WebKit/chromium/src/js/Tests.js:1872
 +                      function(commandResult) {
Use evaluateInConsole_ instead.
Comment 3 Alexander Pavlov (apavlov) 2010-07-07 08:45:42 PDT
Created attachment 60736 [details]
[PATCH] Comments addressed
Comment 4 Alexander Pavlov (apavlov) 2010-07-07 09:03:25 PDT
Darin, can you please have a look at this change? You seem to know those ropes...
Comment 5 Darin Fisher (:fishd, Google) 2010-07-07 09:41:31 PDT
I think it would be better to create a stack of PageGroupLoadDeferrer objects.  It is safe to nest instances of those, and that's what we should actually do if we need to support multiple levels of nesting.
Comment 6 Alexander Pavlov (apavlov) 2010-07-07 10:44:31 PDT
Created attachment 60752 [details]
[PATCH] Implemented a stack of PageGroupLoadDeferrers, as suggested by fishd
Comment 7 Alexander Pavlov (apavlov) 2010-07-08 02:55:21 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
        M       WebKit/chromium/src/js/Tests.js
Committed r62774