Fix empty href for favicon link element causing page to be fetched twice.

Fix empty href for favicon link element causing page to be fetched twice.

Accroding to HTML spec
https://html.spec.whatwg.org/multipage/semantics.html#the-link-element, if the
href attribute in link element is empty, then the element does not define a link
and should be ignored.

BUG=669351
Committed: https://crrev.com/24d87e2f1a18e9cc58a37126b85c921ea37ba695
Cr-Commit-Position: refs/heads/master@{#438762}

[lpy, 2016-12-15 00:31:52]

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

[commit-bot: I haz the power, 2016-12-15 00:33:08]

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, 2016-12-15 00:40:43]

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

[commit-bot: I haz the power, 2016-12-15 00:40:44]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[lpy, 2016-12-15 00:53:09]

Patchset #1 (id:1) has been deleted

[lpy, 2016-12-15 00:54:14]

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

[lpy, 2016-12-15 00:54:14]

PTAL.

I assume I should put the test in html/, but I am not 100% sure where to put it.
Any suggestion?

[tkent, 2016-12-15 01:02:23]

> I assume I should put the test in html/, but I am not 100% sure where to put
it. Any suggestion?

This CL should add core/html/HTMLLinkElementTest.cpp.

[lpy, 2016-12-15 01:14:37]

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

[lpy, 2016-12-15 01:14:44]

On 2016/12/15 01:02:23, tkent wrote:
> > I assume I should put the test in html/, but I am not 100% sure where to put
> it. Any suggestion?
> 
> This CL should add core/html/HTMLLinkElementTest.cpp.

Thanks, ptal.

[commit-bot: I haz the power, 2016-12-15 01:15:10]

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

[tkent, 2016-12-15 01:20:04]

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp (right):

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:11: 
Remove the blank line

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:21: void TearDown()
override { ThreadState::current()->collectAllGarbage(); }
We don't need collectAllGarbage() here.  There are no tests sensitive to GC.

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:37:
document().view()->updateAllLifecyclePhases();
updateAllLifecyclePhases() is unnecessary.  Then,
HTMLLinkElementTest::setHtmlInnerHTML() would be unnecessary and setInnerHTML()
should be called directly.

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:42:
TEST_F(HTMLLinkElementTest, href) {
The test name should be more specific.  e.g. TEST_F(HTMLLinkElementTest,
EmptyHrefAttribute)

[commit-bot: I haz the power, 2016-12-15 01:21:20]

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

[commit-bot: I haz the power, 2016-12-15 01:21:21]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[lpy, 2016-12-15 01:25:00]

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

[commit-bot: I haz the power, 2016-12-15 01:25:18]

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

[lpy, 2016-12-15 01:25:39]

Thanks, I updated the patch, ptal.

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp (right):

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:11: 
On 2016/12/15 01:20:03, tkent wrote:
> Remove the blank line

Done.

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:21: void TearDown()
override { ThreadState::current()->collectAllGarbage(); }
On 2016/12/15 01:20:03, tkent wrote:
> We don't need collectAllGarbage() here.  There are no tests sensitive to GC.
> 

Done.

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:37:
document().view()->updateAllLifecyclePhases();
On 2016/12/15 01:20:03, tkent wrote:
> updateAllLifecyclePhases() is unnecessary.  Then,
> HTMLLinkElementTest::setHtmlInnerHTML() would be unnecessary and
setInnerHTML()
> should be called directly.

Done.

https://codereview.chromium.org/2575273002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:42:
TEST_F(HTMLLinkElementTest, href) {
On 2016/12/15 01:20:03, tkent wrote:
> The test name should be more specific.  e.g. TEST_F(HTMLLinkElementTest,
> EmptyHrefAttribute)

Done.

[tkent, 2016-12-15 01:29:11]

https://codereview.chromium.org/2575273002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp (right):

https://codereview.chromium.org/2575273002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:19: void TearDown()
override {}
Remove this line.  You don't need to override it.

[lpy, 2016-12-15 01:30:23]

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

[lpy, 2016-12-15 01:30:36]

https://codereview.chromium.org/2575273002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp (right):

https://codereview.chromium.org/2575273002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp:19: void TearDown()
override {}
On 2016/12/15 01:29:11, tkent wrote:
> Remove this line.  You don't need to override it.

Done.

[commit-bot: I haz the power, 2016-12-15 01:30:48]

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, 2016-12-15 01:36:23]

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

[commit-bot: I haz the power, 2016-12-15 01:36:24]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[lpy, 2016-12-15 04:45:29]

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

[commit-bot: I haz the power, 2016-12-15 04:45:49]

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

[tkent, 2016-12-15 05:28:18]

lgtm

[commit-bot: I haz the power, 2016-12-15 05:38:00]

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

[commit-bot: I haz the power, 2016-12-15 05:38:01]

Dry run: This issue passed the CQ dry run.

[lpy, 2016-12-15 05:49:47]

The CQ bit was checked by lpy@chromium.org

[commit-bot: I haz the power, 2016-12-15 05:50:10]

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, 2016-12-15 05:54:50]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481780987984720,
"parent_rev": "b88a832c470691605b477b55426216316ef93a0b", "commit_rev":
"700d8b1e119ba57653245c2f4d282c9b10be38c5"}

[commit-bot: I haz the power, 2016-12-15 05:55:18]

Description was changed from

==========
Fix empty href for favicon link element causing page to be fetched twice.

Accroding to HTML spec
https://html.spec.whatwg.org/multipage/semantics.html#the-link-element, if the
href attribute in link element is empty, then the element does not define a link
and should be ignored.

BUG=669351
==========

to

==========
Fix empty href for favicon link element causing page to be fetched twice.

Accroding to HTML spec
https://html.spec.whatwg.org/multipage/semantics.html#the-link-element, if the
href attribute in link element is empty, then the element does not define a link
and should be ignored.

BUG=669351

Review-Url: https://codereview.chromium.org/2575273002
==========

[commit-bot: I haz the power, 2016-12-15 05:55:19]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-12-15 05:59:12]

Description was changed from

==========
Fix empty href for favicon link element causing page to be fetched twice.

Accroding to HTML spec
https://html.spec.whatwg.org/multipage/semantics.html#the-link-element, if the
href attribute in link element is empty, then the element does not define a link
and should be ignored.

BUG=669351

Review-Url: https://codereview.chromium.org/2575273002
==========

to

==========
Fix empty href for favicon link element causing page to be fetched twice.

Accroding to HTML spec
https://html.spec.whatwg.org/multipage/semantics.html#the-link-element, if the
href attribute in link element is empty, then the element does not define a link
and should be ignored.

BUG=669351

Committed: https://crrev.com/24d87e2f1a18e9cc58a37126b85c921ea37ba695
Cr-Commit-Position: refs/heads/master@{#438762}
==========

[commit-bot: I haz the power, 2016-12-15 05:59:13]

Patchset 4 (id:??) landed as
https://crrev.com/24d87e2f1a18e9cc58a37126b85c921ea37ba695
Cr-Commit-Position: refs/heads/master@{#438762}