[devtools] Remove trailing fragment identifier from script urls

Source/bindings/v8/ScriptDebugServer.cpp

 /*
  * Copyright (c) 2010-2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 484 matching lines @@
             handleProgramBreak(eventDetails, v8::Handle<v8::Value>(), hitBreakpoints.As<v8::Array>());
         }
     }
 }
 
 void ScriptDebugServer::dispatchDidParseSource(ScriptDebugListener* listener, v8::Handle<v8::Object> object)
 {
     String sourceID = toWebCoreStringWithUndefinedOrNullCheck(object->Get(v8::String::NewSymbol("id")));
 
     ScriptDebugListener::Script script;
-    script.url = toWebCoreStringWithUndefinedOrNullCheck(object->Get(v8::String::NewSymbol("name")));
+
+    String rawScriptURL =  toWebCoreStringWithUndefinedOrNullCheck(object->Get(v8::String::NewSymbol("name")));
+    // Remove any fragment identifier.
+    size_t hashCharPos = rawScriptURL.find("#");

[abarth-chromium, 2013/10/11 20:20:41]

This seems like the wrong layer for this work.  The job of the bindings layer is
just to do type conversions.  All "application logic" should be handled outside
the bindings.

Also, shouldn't we use a real URL parser?

[johnjbarton, 2013/10/11 21:31:16]

The debugger layer (listener here) gets a const Script&. I could change
didParseSource() to take Script&, but that seems wrong.

The other place I can fix it is before we compile. Two classes represent JS it
seems, one is ScriptSourceCode but that is still in bindings. The other is
WebScriptSource. Construction of WebScriptSource from a string and a URL could
remove the fragment identifier.
WebScriptSource has a WebURL with a path to GURL and GURL::PathForRequest()
which computes the string we would want to use for scripts.

+    if (hashCharPos != kNotFound) {
+        script.url = rawScriptURL.substring(0, hashCharPos);
+    } else {
+        script.url = rawScriptURL;
+    }

[abarth-chromium, 2013/10/11 20:20:41]

This branch isn't needed.  substring(0, kNotFound) will just return the whole
string without making a copy.

[johnjbarton, 2013/10/11 21:31:16]

If I understand you, I could have written:
   script.url = script.url.substring(0, script.url.find("#"));
?

+
     script.source = toWebCoreStringWithUndefinedOrNullCheck(object->Get(v8::String::NewSymbol("source")));
     script.sourceMappingURL = toWebCoreStringWithUndefinedOrNullCheck(object->Get(v8::String::NewSymbol("sourceMappingURL")));
     script.startLine = object->Get(v8::String::NewSymbol("startLine"))->ToInteger()->Value();
     script.startColumn = object->Get(v8::String::NewSymbol("startColumn"))->ToInteger()->Value();
     script.endLine = object->Get(v8::String::NewSymbol("endLine"))->ToInteger()->Value();
     script.endColumn = object->Get(v8::String::NewSymbol("endColumn"))->ToInteger()->Value();
     script.isContentScript = object->Get(v8::String::NewSymbol("isContentScript"))->ToBoolean()->Value();
 
     listener->didParseSource(sourceID, script);
 }
@@ skipping 112 matching lines @@
 {
     return PassOwnPtr<ScriptSourceCode>();
 }
 
 String ScriptDebugServer::preprocessEventListener(Frame*, const String& source, const String& url, const String& functionName)
 {
     return source;
 }
 
 } // namespace WebCore