Fix hard-coding of "about:srcdoc"

Fix hard-coding of "about:srcdoc"

BUG=555325
Committed: https://crrev.com/7461ceb14aebe911fd453d2ee54c261a8be3dc59
Cr-Commit-Position: refs/heads/master@{#362933}

[Xiaocheng, 2015-11-13 03:36:17]

Description was changed from

==========
Fix hard-coding of "about:srcdoc"

BUG=555325
==========

to

==========
Fix hard-coding of "about:srcdoc"

BUG=555325
==========

[Xiaocheng, 2015-11-13 03:36:17]

xiaochengh@chromium.org changed reviewers:
+ dominicc@chromium.org, japhet@chromium.org, tsepez@chromium.org

[Xiaocheng, 2015-11-13 03:37:30]

PTAL.

[dominicc (has gone to gerrit), 2015-11-16 07:50:30]

lgtm

One nit inline.

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/weborigin/KURL.h (right):

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/weborigin/KURL.h:141: bool isAboutSrcdocURL()
const; // Is exactly about:srcdoc
Nit: The previous line's comment ends with a period. Be consistent, otherwise
you might give the impression that about:blank ends with a period or something.

[Tom Sepez, 2015-11-16 17:16:06]

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:1461: if
(!equalIgnoringCase(url, srcdocURL()))
Shouldn't this be url.isAboutSrcDocUrl() ? We now have pair of KURL objects
here, it seems like we should be using a KURL method to make this comparison,
rather than letting them both implicitly convert to String and then using a
String method. Does KURL's normalization take care of the case issue for you?

[Xiaocheng, 2015-11-17 05:25:47]

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:1461: if
(!equalIgnoringCase(url, srcdocURL()))
On 2015/11/16 17:16:06, Tom Sepez wrote:
> Shouldn't this be url.isAboutSrcDocUrl() ? We now have pair of KURL objects
> here, it seems like we should be using a KURL method to make this comparison,
> rather than letting them both implicitly convert to String and then using a
> String method. Does KURL's normalization take care of the case issue for you?

Done.

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/weborigin/KURL.h (right):

https://codereview.chromium.org/1444493002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/weborigin/KURL.h:141: bool isAboutSrcdocURL()
const; // Is exactly about:srcdoc
On 2015/11/16 07:50:30, dominicc wrote:
> Nit: The previous line's comment ends with a period. Be consistent, otherwise
> you might give the impression that about:blank ends with a period or
something.

Done.

[Tom Sepez, 2015-11-17 17:17:44]

lgtm

[dominicc (has gone to gerrit), 2015-11-26 03:12:24]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2015-11-26 03:12:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/1444493002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-11-26 03:12:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1444493002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1444493002/40001

[commit-bot: I haz the power, 2015-11-26 05:14:42]

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

[commit-bot: I haz the power, 2015-11-26 05:14:43]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[dominicc (has gone to gerrit), 2015-12-03 06:44:00]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2015-12-03 06:44:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1444493002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1444493002/40001

[commit-bot: I haz the power, 2015-12-03 08:08:55]

Description was changed from

==========
Fix hard-coding of "about:srcdoc"

BUG=555325
==========

to

==========
Fix hard-coding of "about:srcdoc"

BUG=555325
==========

[commit-bot: I haz the power, 2015-12-03 08:08:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-03 08:09:54]

Description was changed from

==========
Fix hard-coding of "about:srcdoc"

BUG=555325
==========

to

==========
Fix hard-coding of "about:srcdoc"

BUG=555325

Committed: https://crrev.com/7461ceb14aebe911fd453d2ee54c261a8be3dc59
Cr-Commit-Position: refs/heads/master@{#362933}
==========

[commit-bot: I haz the power, 2015-12-03 08:09:54]

Patchset 3 (id:??) landed as
https://crrev.com/7461ceb14aebe911fd453d2ee54c261a8be3dc59
Cr-Commit-Position: refs/heads/master@{#362933}