[Webkit-unassigned] [Bug 40300] Web Inspector: [JSC] implement script source editing

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 6 07:12:44 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=40300





--- Comment #47 from Yong Li <yoli at rim.com>  2012-07-06 07:12:42 PST ---
(In reply to comment #45)
> (In reply to comment #44)
> > (In reply to comment #40)
> > > (In reply to comment #38)
> > > > Why isn't justRecompile the last argument here?
> > > Because the last two parameters have default value, if we pull justRecompile in last position we have to set a default value otherwise it causes a compiling error. 
> > 
> > Then we can give a default value? or move arg to the similar position for other functions? It is better to see consistent order.
> 
> Yes, but there is a trivial reason causes we cannot do that. Firstly, we have to keep the old interface untouched:
> evaluate(ExecState*, ScopeChainNode*, const SourceCode&, JSValue thisValue = JSValue(), JSValue* exception = 0);
> Otherwise we will break the windows porting since it's a interface of DLL.
> Then if we put "compileOnly" in last position with a default value, looks like this: 
> evaluate(ExecState*, ScopeChainNode*, const SourceCode&, JSValue thisValue = JSValue(), JSValue* exception = 0, bool compileOnly = false);
> The compiler will confused it with the old interface and complain. 

Can't we change the DLL interface or ask Apple gentlemen for help?

> > > > > Source/WebCore/bindings/js/ScriptDebugServer.cpp:201
> > > > >  bool ScriptDebugServer::canSetScriptSource()
> > > > >  {
> > > > > -    return false;
> > > > > +    return true;
> > > > >  }
> > > > 
> > > > why return true here?
> > > It's a legacy code, since JSC doesn't support this feature but V8 support. Inpsector uses this interface to probe it. If this bug is fixed, this function plus related JS code of front-end will be removed in a dependent bug report.
> > 
> > I see we are using PageScriptDebugServer. Can we make it virtual and leave the base class to return false?
> Don't worry this function too much. It will be removed soon after this bug fixed, like what we did in bug#88759 

We shouldn't push unfinished thing into the repo. Logically ScriptDebugServer doesn't support setScriptSource() by itself.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list