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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 5 07:22:30 PDT 2012


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





--- Comment #44 from Yong Li <yoli at rim.com>  2012-07-05 07:22:29 PST ---
(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.

> > > 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?

> > > Source/WebCore/bindings/js/ScriptDebugServer.cpp:206
> > >  bool ScriptDebugServer::setScriptSource(const String&, const String&, bool, String*, ScriptValue*, ScriptObject*)
> > >  {
> > > -    // FIXME(40300): implement this.
> > > -    return false;
> > > +    return true;
> > >  }
> > 
> > why return true here but nothing is done?
> This function is never be invoked. Its override in PageScriptDebugServer will play the rule. For JSC we cannot implement the feature at "ScriptDebugServer" level as V8 does, since V8 seems have a extra shell layer to record the origin of JS source code.

If it is never invoked, why do we change it? A pure virtual function or ASSERT_NOT_REACHED may be better.

> > 
> > should be SourceIDToGlobalObjectMap?
> sure, thx. 

m_sourceIdToGlobalObjectMap is still there :)

-- 
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