Bug 41697 - [Chromium] Crash when re-entering message loop
Summary: [Chromium] Crash when re-entering message loop
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
Depends on:
Reported: 2010-07-06 07:20 PDT by Alexander Pavlov (apavlov)
Modified: 2010-07-08 02:55 PDT (History)
4 users (show)

See Also:

[PATCH] Suggested solution (4.19 KB, patch)
2010-07-06 08:20 PDT, Alexander Pavlov (apavlov)
yurys: review-
Details | Formatted Diff | Diff
[PATCH] Comments addressed (5.38 KB, patch)
2010-07-07 08:45 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Implemented a stack of PageGroupLoadDeferrers, as suggested by fishd (7.07 KB, patch)
2010-07-07 10:44 PDT, Alexander Pavlov (apavlov)
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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

 +  static PageGroupLoadDeferrer* pageGroupLoadDeferrer = 0;
You don't need explicit initialization here.

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

 +          ASSERT(pageGroupLoadDeferrer);
You may want to check in debug mode that PageGroup hasn't changed since previous call to this method

 +      WebInspector.console.visible = true;
Use  WebInspector.showConsole instead.

 +          function(callFrames) {
Please use evaluateInConsole_ instead of this custom code.

 +                      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