Bug 81970

Summary: Web Inspector: x-frame security errors logged when typing in the console are annoying.
Product: WebKit Reporter: David Levin <levin>
Component: Web Inspector (Deprecated)Assignee: Vsevolod Vlasov <vsevik>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, ossy, paulirish, pfeldman, pmuellr, rik, timothy, vsevik, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 84341    
Bug Blocks:    
Attachments:
Description Flags
outer frame
none
inner frame
none
Patch
none
Patch
none
Patch
none
Patch none

Description David Levin 2012-03-22 14:42:59 PDT
Created attachment 133350 [details]
outer frame

1. Save the two html documents to your local hard drive.
2. open test_iframe.html
3. open inspector and put a breakpoint in foo()
4. in the console type "window.frameElement" (Don't paste it. Actually type it.) or "window.parent.location", etc.
Note that you get lots of "Unsafe JavaScript attempt to access frame with URL" errors in the console output as you type. This makes using the console really painful.

Other:
* These steps as given may only repro with browsers like chromium that treat all local files as different security origins. (You need the inner frame to be a different security origin to repro the issue.)
* In my case, the errors don't collapse together (like they did for me in this simple example) -- perhaps that is because they have a stack trace.
Comment 1 David Levin 2012-03-22 14:43:32 PDT
Created attachment 133351 [details]
inner frame
Comment 2 Vsevolod Vlasov 2012-04-18 08:03:29 PDT
Created attachment 137688 [details]
Patch
Comment 3 Build Bot 2012-04-18 08:27:10 PDT
Comment on attachment 137688 [details]
Patch

Attachment 137688 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12427225
Comment 4 Pavel Feldman 2012-04-18 08:31:34 PDT
Comment on attachment 137688 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137688&action=review

> Source/WebCore/inspector/PageRuntimeAgent.cpp:72
> +    Frame* mainFrame = m_inspectedPage->mainFrame();

I would use a counter here.

> Source/WebCore/inspector/PageRuntimeAgent.cpp:74
> +    for (Frame* frame = mainFrame; frame; frame = frame->tree()->traverseNext(mainFrame))

I would use a static counter on console instead.
Comment 5 Vsevolod Vlasov 2012-04-18 09:44:43 PDT
Created attachment 137711 [details]
Patch
Comment 6 Build Bot 2012-04-18 10:13:58 PDT
Comment on attachment 137711 [details]
Patch

Attachment 137711 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12429239
Comment 7 Vsevolod Vlasov 2012-04-18 11:07:52 PDT
Created attachment 137725 [details]
Patch
Comment 8 Pavel Feldman 2012-04-19 03:26:08 PDT
Comment on attachment 137725 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137725&action=review

> Source/WebCore/page/Console.cpp:59
> +    int s_muteCount = 0;

Either declare it in the Console.h or stop using the s_ prefix.
Comment 9 Vsevolod Vlasov 2012-04-19 06:28:46 PDT
Committed r114628: <http://trac.webkit.org/changeset/114628>
Comment 10 Csaba Osztrogonác 2012-04-19 07:38:22 PDT
Reopen, because it broke the debug build on Qt:
../../../../Source/WebCore/inspector/InspectorDebuggerAgent.cpp: In member function ‘virtual void WebCore::InspectorDebuggerAgent::evaluateOnCallFrame(WebCore::ErrorString*, const WTF::String&, const WTF::String&, const WTF::String*, const bool*, const bool*, const bool*, WTF::RefPtr<WebCore::TypeBuilder::Runtime::RemoteObject>&, WebCore::TypeBuilder::OptOutput<bool>*)’:
../../../../Source/WebCore/inspector/InspectorDebuggerAgent.cpp:492: error: no match for ‘operator!’ in ‘!WebCore::InspectorDebuggerAgent::scriptDebugServer()’
../../../../Source/WebCore/inspector/InspectorDebuggerAgent.cpp:492: note: candidates are: operator!(bool) <built-in>
Comment 11 Vsevolod Vlasov 2012-04-19 07:54:41 PDT
Created attachment 137901 [details]
Patch
Comment 12 Vsevolod Vlasov 2012-04-19 08:34:48 PDT
Committed r114632: <http://trac.webkit.org/changeset/114632>
Comment 13 Timothy Hatcher 2012-04-26 18:18:16 PDT
Renaming the existing doNotPauseOnExceptions parameter to doNotPauseOnExceptionsAndMuteConsole breaks backward compatibility with version 1.0 of the protocol. Renames should not be allowed.

Am I correct? If so I'll file a bug about making the compatibility checking catch such things.
Comment 14 Pavel Feldman 2012-04-27 04:46:50 PDT
(In reply to comment #13)
> Renaming the existing doNotPauseOnExceptions parameter to doNotPauseOnExceptionsAndMuteConsole breaks backward compatibility with version 1.0 of the protocol. Renames should not be allowed.
> 
> Am I correct? If so I'll file a bug about making the compatibility checking catch such things.

We do not guarantee compatibility of the hidden members (methods, parameters, etc). It is explicitly stated in the announcement and these members are missing in the documentation.
Comment 15 Timothy Hatcher 2012-04-27 07:06:08 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > Renaming the existing doNotPauseOnExceptions parameter to doNotPauseOnExceptionsAndMuteConsole breaks backward compatibility with version 1.0 of the protocol. Renames should not be allowed.
> > 
> > Am I correct? If so I'll file a bug about making the compatibility checking catch such things.
> 
> We do not guarantee compatibility of the hidden members (methods, parameters, etc). It is explicitly stated in the announcement and these members are missing in the documentation.

Sorry, I overlooked the fact that it was hidden. Too many things are hidden, so the protocol is pretty useless without using hidden members.

What is your process and timeframe for unhiding minor things like this?
Comment 16 Pavel Feldman 2012-05-02 02:08:06 PDT
> What is your process and timeframe for unhiding minor things like this?

There is really no process / schedule for that. You should probably file bugs for such changes.