Replace the type of hint url for blink::detectTextEncoding

Replace the type of hint url for blink::detectTextEncoding

This helps the method avoid creating a new instance of KURL
every time it is invoked.

BUG=704800
Review-Url: https://codereview.chromium.org/2786913002
Cr-Commit-Position: refs/heads/master@{#461355}
Committed: https://chromium.googlesource.com/chromium/src/+/c93c2e8927446064827541e3b029d7312686dfd1

[Jinsuk Kim, 2017-03-30 07:03:42]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 07:03:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 07:46:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 07:46:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jinsuk Kim, 2017-03-30 12:45:14]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 12:45:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 14:47:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 14:47:46]

Dry run: This issue passed the CQ dry run.

[Jinsuk Kim, 2017-03-30 22:26:48]

jinsukkim@chromium.org changed reviewers:
+ tkent@chromium.org

[tkent, 2017-03-30 23:23:59]

https://codereview.chromium.org/2786913002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/text/TextEncodingDetector.h (right):

https://codereview.chromium.org/2786913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/text/TextEncodingDetector.h:54: const KURL*
hintUrl,
Make the argument |const KURL&|.
KURL has a null state. We don't need to use a pointer here.

[Jinsuk Kim, 2017-03-31 00:39:08]

https://codereview.chromium.org/2786913002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/text/TextEncodingDetector.h (right):

https://codereview.chromium.org/2786913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/text/TextEncodingDetector.h:54: const KURL*
hintUrl,
On 2017/03/30 23:23:59, tkent wrote:
> Make the argument |const KURL&|.
> KURL has a null state. We don't need to use a pointer here.

Done.

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

lgtm

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h (right):

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h:141: const
KURL& m_hintUrl;
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:30]

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h (right):

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h:141: const
KURL& m_hintUrl;
On 2017/03/31 00:47:30, tkent wrote:
> This should be |const KURL|, not |const KURL&|. The current code holds a
> reference to a temporary object.

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]

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h (right):

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h:141: const
KURL& m_hintUrl;
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:44:30]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-02 22:44:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h (right):

https://codereview.chromium.org/2786913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/TextResourceDecoder.h:141: const
KURL& m_hintUrl;
On 2017/03/31 08:12:24, tkent wrote:
> I think this CL is still worth to land.  Copying cost of KRUL isn't large, and
> the code becomes cleaner.

Done.

[commit-bot: I haz the power, 2017-04-03 00:23:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 00:23:02]

Dry run: This issue passed the CQ dry run.

[Jinsuk Kim, 2017-04-03 01:02:40]

The CQ bit was checked by jinsukkim@chromium.org

[Jinsuk Kim, 2017-04-03 01:02:40]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/2786913002/#ps60001
(title: "const KURL")

[commit-bot: I haz the power, 2017-04-03 01:02:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 01:07:30]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491181360067240,
"parent_rev": "b8e4332ca05f3220a36557b0e25dd3fa75365672", "commit_rev":
"c93c2e8927446064827541e3b029d7312686dfd1"}

[commit-bot: I haz the power, 2017-04-03 01:08:38]

Description was changed from

==========
Replace the type of hint url for blink::detectTextEncoding

This helps the method avoid creating a new instance of KURL
every time it is invoked.

BUG=704800
==========

to

==========
Replace the type of hint url for blink::detectTextEncoding

This helps the method avoid creating a new instance of KURL
every time it is invoked.

BUG=704800

Review-Url: https://codereview.chromium.org/2786913002
Cr-Commit-Position: refs/heads/master@{#461355}
Committed:
https://chromium.googlesource.com/chromium/src/+/c93c2e8927446064827541e3b029...
==========

[commit-bot: I haz the power, 2017-04-03 01:08:39]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/c93c2e8927446064827541e3b029...