RESOLVED FIXED 36612
Web Inspector: don't crash when there are messages in console from previous iframe content
https://bugs.webkit.org/show_bug.cgi?id=36612
Summary Web Inspector: don't crash when there are messages in console from previous i...
Yury Semikhatsky
Reported 2010-03-25 11:25:34 PDT
Web Inspector sometimes crashes when there are messages written to the console from iframe which has navigated to another domain by the time Web Inspector is open.
Attachments
patch (21.68 KB, patch)
2010-03-26 09:18 PDT, Yury Semikhatsky
no flags
patch (25.04 KB, patch)
2010-03-26 09:30 PDT, Yury Semikhatsky
pfeldman: review+
Yury Semikhatsky
Comment 1 2010-03-26 09:18:45 PDT
WebKit Review Bot
Comment 2 2010-03-26 09:23:29 PDT
Attachment 51747 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/bindings/v8/ScriptState.h:91: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yury Semikhatsky
Comment 3 2010-03-26 09:30:48 PDT
Created attachment 51749 [details] patch Reformatted ScriptState.h to make style checker happy.
Pavel Feldman
Comment 4 2010-03-28 05:20:09 PDT
Comment on attachment 51749 [details] patch r+ with comments. please fix prior to landing. > +CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector-enabled/resources/console-log-frame-after-navigation.html from frame with URL http://127.0.0.1:8000/inspector-enabled/resources/console-log-frame-before-navigation.html. Domains, protocols and ports must match. I am not sure this will be an exact match on all platforms including chromium. Please test that. > +InjectedScript._toString = function(obj) > +{ > + // We don't use String(obj) because inspectedWindow.String is undefined if owning frame navigated to another page. > + return "" + obj; This looks like a hack. Does it happen in both engines? Should you file a bug against it and put fixme here? Otherwise looks like total hack.
Yury Semikhatsky
Comment 5 2010-03-29 00:07:48 PDT
(In reply to comment #4) > (From update of attachment 51749 [details]) > r+ with comments. please fix prior to landing. > > > +CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector-enabled/resources/console-log-frame-after-navigation.html from frame with URL http://127.0.0.1:8000/inspector-enabled/resources/console-log-frame-before-navigation.html. Domains, protocols and ports must match. > > I am not sure this will be an exact match on all platforms including chromium. > Please test that. > This message doesn't appear in Chromium and looks as a bug to me because postMessage is designed for cross domain communication. We will need custom test expectations for chromium. I filed a bug on this: https://bugs.webkit.org/show_bug.cgi?id=36740 > > +InjectedScript._toString = function(obj) > > +{ > > + // We don't use String(obj) because inspectedWindow.String is undefined if owning frame navigated to another page. > > + return "" + obj; > > This looks like a hack. Does it happen in both engines? It's a workaround for JSC only, in v8 inspectedWindow.String is still defined after frame navigation.
Yury Semikhatsky
Comment 6 2010-03-29 00:35:03 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/http/tests/inspector-enabled/console-log-before-frame-navigation-expected.txt A LayoutTests/http/tests/inspector-enabled/console-log-before-frame-navigation.html A LayoutTests/http/tests/inspector-enabled/resources/console-log-before-frame-navigation.js A LayoutTests/http/tests/inspector-enabled/resources/console-log-frame-after-navigation.html A LayoutTests/http/tests/inspector-enabled/resources/console-log-frame-before-navigation.html M WebCore/ChangeLog M WebCore/bindings/js/ScriptState.h M WebCore/bindings/v8/ScriptState.h M WebCore/inspector/ConsoleMessage.cpp M WebCore/inspector/ConsoleMessage.h M WebCore/inspector/front-end/InjectedScript.js Committed r56708
Note You need to log in before you can comment on or make changes to this bug.