Replace the type of hint url for blink::detectTextEncoding

third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h

 /*
     Copyright (C) 1999 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
     Copyright (C) 2006, 2008 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
     version 2 of the License, or (at your option) any later version.
 
     This library is distributed in the hope that it will be useful,
     but WITHOUT ANY WARRANTY; without even the implied warranty of
     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
     Library General Public License for more details.
 
     You should have received a copy of the GNU Library General Public License
     along with this library; see the file COPYING.LIB.  If not, write to
     the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
     Boston, MA 02110-1301, USA.
 
 */
 
 #ifndef TextResourceDecoder_h
 #define TextResourceDecoder_h
 
+#include <memory>
 #include "core/CoreExport.h"
+#include "platform/weborigin/KURL.h"
 #include "wtf/PtrUtil.h"
 #include "wtf/text/TextEncoding.h"
-#include <memory>
 
 namespace blink {
 
 class HTMLMetaCharsetParser;
 
 class CORE_EXPORT TextResourceDecoder {
   USING_FAST_MALLOC(TextResourceDecoder);
   WTF_MAKE_NONCOPYABLE(TextResourceDecoder);
 
  public:
   enum EncodingSource {
     DefaultEncoding,
     AutoDetectedEncoding,
     EncodingFromContentSniffing,
     EncodingFromXMLHeader,
     EncodingFromMetaTag,
     EncodingFromCSSCharset,
     EncodingFromHTTPHeader,
     EncodingFromParentFrame
   };
 
   static std::unique_ptr<TextResourceDecoder> create(
       const String& mimeType,
       const WTF::TextEncoding& defaultEncoding = WTF::TextEncoding()) {
     return WTF::wrapUnique(new TextResourceDecoder(
-        mimeType, defaultEncoding, UseContentAndBOMBasedDetection, String()));
+        mimeType, defaultEncoding, UseContentAndBOMBasedDetection, KURL()));
   }
 
   static std::unique_ptr<TextResourceDecoder> createWithAutoDetection(
       const String& mimeType,
       const WTF::TextEncoding& defaultEncoding,
-      const String& url) {
+      const KURL& url) {
     return WTF::wrapUnique(new TextResourceDecoder(mimeType, defaultEncoding,
                                                    UseAllAutoDetection, url));
   }
 
   // Corresponds to utf-8 decode in Encoding spec:
   // https://encoding.spec.whatwg.org/#utf-8-decode.
   static std::unique_ptr<TextResourceDecoder> createAlwaysUseUTF8ForText() {
     return WTF::wrapUnique(new TextResourceDecoder(
-        "plain/text", UTF8Encoding(), AlwaysUseUTF8ForText, String()));
+        "plain/text", UTF8Encoding(), AlwaysUseUTF8ForText, KURL()));
   }
   ~TextResourceDecoder();
 
   void setEncoding(const WTF::TextEncoding&, EncodingSource);
   const WTF::TextEncoding& encoding() const { return m_encoding; }
   bool encodingWasDetectedHeuristically() const {
     return m_source == AutoDetectedEncoding ||
            m_source == EncodingFromContentSniffing;
   }
 
@@ skipping 25 matching lines @@
     // |m_contentType| must be |PlainTextContent| and
     // |m_encoding| must be UTF8Encoding.
     // This doesn't change encoding based on BOMs, but still processes
     // utf-8 BOMs so that utf-8 BOMs don't appear in the decoded result.
     AlwaysUseUTF8ForText
   };
 
   TextResourceDecoder(const String& mimeType,
                       const WTF::TextEncoding& defaultEncoding,
                       EncodingDetectionOption,
-                      const String& url);
+                      const KURL& hintUrl);
 
  private:
   enum ContentType {
     PlainTextContent,
     HTMLContent,
     XMLContent,
     CSSContent
   };  // PlainText only checks for BOM.
   static ContentType determineContentType(const String& mimeType);
   static const WTF::TextEncoding& defaultEncoding(
       ContentType,
       const WTF::TextEncoding& defaultEncoding);
 
   bool checkForCSSCharset(const char*, size_t, bool& movedDataToBuffer);
   bool checkForXMLCharset(const char*, size_t, bool& movedDataToBuffer);
   void checkForMetaCharset(const char*, size_t);
   bool shouldAutoDetect() const;
 
   ContentType m_contentType;
   WTF::TextEncoding m_encoding;
   std::unique_ptr<TextCodec> m_codec;
   EncodingSource m_source;
   const char* m_hintEncoding;
-  const CString m_hintUrl;
+  const KURL& m_hintUrl;

[tkent, 2017/03/31 00:47:30]

This should be |const KURL|, not |const KURL&|. The current code holds a
reference to a temporary object.

[Jinsuk Kim, 2017/03/31 01:33:29]

Oops. You're correct. Then we have to create a new KURL instance for every
TextResourceDecoder... In fact this is worse because previously only those TRDs
that do auto encoding detection create one (auto detection is called just once
per TRD). Do you think it's still worth landing this CL, even if
TextResourceDecoder instantiates a new KURL? It was the whole point of this CL
from my POV.

One thing I'd suggest is: given that such problem happens mostly for tests only,
how about define a a static immutable null KURL instance and pass it instead of
a temporary object here?

If you believe this CL still has value even if every TextResourceDecoder
instantiates a new KURL, I'll go ahead and land it after changing the type to
|const KURL|.

[tkent, 2017/03/31 08:12:24]

I think this CL is still worth to land.  Copying cost of KRUL isn't large, and
the code becomes cleaner.

[Jinsuk Kim, 2017/04/02 22:46:10]

Done.

   Vector<char> m_buffer;
   char m_hintLanguage[3];
   bool m_checkedForBOM;
   bool m_checkedForCSSCharset;
   bool m_checkedForXMLCharset;
   bool m_checkedForMetaCharset;
   bool m_useLenientXMLDecoding;  // Don't stop on XML decoding errors.
   bool m_sawError;
   EncodingDetectionOption m_encodingDetectionOption;
 
   std::unique_ptr<HTMLMetaCharsetParser> m_charsetParser;
 };
 
 }  // namespace blink
 
 #endif