SourceLocation: Fix use-after-move.

third_party/WebKit/Source/bindings/core/v8/SourceLocation.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/core/v8/SourceLocation.h"
 
 #include "bindings/core/v8/V8BindingMacros.h"
 #include "bindings/core/v8/V8PerIsolateData.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExecutionContext.h"
@@ skipping 23 matching lines @@
     ScriptForbiddenScope::AllowUserAgentScript allowScripting;
     return data->threadDebugger()->debugger()->captureStackTrace(stackSize);
 }
 
 }
 
 // static
 PassOwnPtr<SourceLocation> SourceLocation::capture(const String& url, unsigned lineNumber, unsigned columnNumber)
 {
     std::unique_ptr<V8StackTrace> stackTrace = captureStackTrace();
-    if (stackTrace && !stackTrace->isEmpty())
-        return SourceLocation::create(stackTrace->topSourceURL(), stackTrace->topLineNumber(), stackTrace->topColumnNumber(), std::move(stackTrace), 0);
+    if (stackTrace && !stackTrace->isEmpty()) {
+        const V8StackTrace* stackTracePtr = stackTrace.get();

[Yuki, 2016/05/26 06:55:50]

I'm a bit uneasy to use a raw pointer here.
    const String& url = stackTrace->topSourceURL();
    const unsigned lineNumber = stackTrace->topLineNumber();
    ...
    return SourceLocation::create(url, lineNumber, ...);
seems more "right things" to me, personally.

Anyway, without a comment, it's not easy to understand the intention.  I'd
propose the following things in the preferred order.

1) Adds SourceLocation::create(std::unique_ptr<V8StackTrace>, int scriptId = 0)
and use it.  No need to pass url, line number, etc. seeing these two cases.
2) Adds a comment to explain the fix.  Why it's needed.
2-1) [optional] Use temporary variables rather than a raw pointer.

It's fine to assign the issue to me if you don't have time.

[kinaba, 2016/05/26 07:14:19]

Makes sense. Done.

+        return SourceLocation::create(stackTracePtr->topSourceURL(), stackTracePtr->topLineNumber(), stackTracePtr->topColumnNumber(), std::move(stackTrace), 0);
+    }
     return SourceLocation::create(url, lineNumber, columnNumber, std::move(stackTrace));
 }
 
 // static
 PassOwnPtr<SourceLocation> SourceLocation::capture(ExecutionContext* executionContext)
 {
     std::unique_ptr<V8StackTrace> stackTrace = captureStackTrace();
-    if (stackTrace && !stackTrace->isEmpty())
-        return SourceLocation::create(stackTrace->topSourceURL(), stackTrace->topLineNumber(), stackTrace->topColumnNumber(), std::move(stackTrace), 0);
+    if (stackTrace && !stackTrace->isEmpty()) {
+        const V8StackTrace* stackTracePtr = stackTrace.get();
+        return SourceLocation::create(stackTracePtr->topSourceURL(), stackTracePtr->topLineNumber(), stackTracePtr->topColumnNumber(), std::move(stackTrace), 0);
+    }
 
     Document* document = executionContext && executionContext->isDocument() ? toDocument(executionContext) : nullptr;
     if (document) {
         unsigned lineNumber = 0;
         if (document->scriptableDocumentParser() && !document->isInDocumentWrite()) {
             if (document->scriptableDocumentParser()->isParsingAtLineNumber())
                 lineNumber = document->scriptableDocumentParser()->lineNumber().oneBasedInt();
         }
         return SourceLocation::create(document->url().getString(), lineNumber, 0, std::move(stackTrace));
     }
@@ skipping 17 matching lines @@
             scriptId = 0;
     }
 
     int lineNumber = 0;
     int columnNumber = 0;
     if (v8Call(message->GetLineNumber(isolate->GetCurrentContext()), lineNumber)
         && v8Call(message->GetStartColumn(isolate->GetCurrentContext()), columnNumber))
         ++columnNumber;
 
     if ((!scriptId || !lineNumber) && stackTrace && !stackTrace->isEmpty())
         return adoptPtr(new SourceLocation(stackTrace->topSourceURL(), stackTrace->topLineNumber(), stackTrace->topColumnNumber(), std::move(stackTrace), 0));

[Yuki, 2016/05/26 07:05:36]

This line also seems problematic.

[kinaba, 2016/05/26 07:14:19]

Good catch!

 
     String url = toCoreStringWithUndefinedOrNullCheck(message->GetScriptOrigin().ResourceName());
     if (url.isNull())
         url = executionContext->url();
     return adoptPtr(new SourceLocation(url, lineNumber, columnNumber, std::move(stackTrace), scriptId));
 }
 
 // static
 PassOwnPtr<SourceLocation> SourceLocation::create(const String& url, unsigned lineNumber, unsigned columnNumber, std::unique_ptr<V8StackTrace> stackTrace, int scriptId)
 {
@@ skipping 27 matching lines @@
     value->endDictionary();
     value->endArray();
 }
 
 PassOwnPtr<SourceLocation> SourceLocation::clone() const
 {
     return adoptPtr(new SourceLocation(m_url, m_lineNumber, m_columnNumber, m_stackTrace ? m_stackTrace->clone() : nullptr, m_scriptId));
 }
 
 } // namespace blink