I'm pretty sure this is due to mkwst@'s r462933.  All three tests are missing "CONSOLE WARNING: Resource requests whose URLs contain raw newline characters are deprecated, and may be blocked in M60, around August 2017. Please remove newlines from places like element attribute values in order to continue loading those resources. See https://www.chromestatus.com/features/5735596811091968 for more details."

r462933 introduced this warning and updated expectations for a bunch of tests that trigger it, including those three tests.  But it looks like we don't see the warning with --site-per-process. 

I see that Deprecation::countDeprecation() is used to output the warning.  One problem with that is the use of page->useCounter().hasRecordedMeasurement() and page->deprecation().m_muteCount, which aren't replicated for OOPIFs.  I can see that leading to extra warnings though, but here there's just one warning which isn't working, so not sure if that's what's causing this issue.

Might be a timing change? All three of those tests navigate to a `data:` URL that fires `testRunner.notifyDone()` in a `setTimeout(..., 0)`. Is it possible that that's firing before the console message is printed? Do you see the message if you comment out the `notifyDone()` call and run the test from the commandline?

Mike - thanks for the suggestion, but unfortunately that doesn't appear to help here.  I still don't see the message with --site-per-process and setTimeout commented out.

After some debugging, what seems to be happening is with --site-per-process, we get into FrameFetchContext::CanRequestInternal, but we never even try to print the deprecation message because url.WhitespaceRemoved() returns false.  The frame being navigated here will be a remote frame with --site-per-process, so we will send the navigation to the browser process, which will ask the OOPIF renderer to navigate (via RenderFrameImpl::OnNavigate), which is where we'll end up calling CanRequestInternal and where url.WhitespaceRemoved() fails.  So perhaps we've lost the WhitespaceRemoved bit when transitioning the URL over IPC?

Maybe we can fix that by additionally making this URL check for proxy navigations in the parent renderer, e.g., in RemoteFrame::Navigate or something on that path.  Or, alternatively, could we check for whitespace somewhere earlier on the load path, where it will work for both local and remote navigations?  Or find a way to preserve the whitespace removed bit?  Mike, WDYT?

(Also, just FYI, I've filed  issue 710637  for the unrelated OOPIF issues with deprecation/use counters I mentioned in #1.)

mkwst@: any thoughts on fix options in #3?

Sorry, I missed this yesterday.

It's very strange that the whitespace bit isn't being preserved in site-per-process mode. IPC is certainly a good place to look for that error. Skimming through the code, it looks like we're indeed just serializing the URL and reparsing it on the other end. Since we've already discarded the whitespace, we lose the flag on the browser side. Bah.

Moving the check for navigations is certainly possible, and is probably the simplest solution, as changing something fundamental like GURL's IPC serialization is something that we'd want to do very carefully. Do you have a particular point in mind? `RenderFrameImpl::OnNavigate` or `RenderFrameImpl::NavigateInternal` seem like they might work, though it's not actually clear to me if those happen in the same renderer process as the initiator or not. I bet you know off the top of your head, though. :)

Today and Monday are holidays, so if you need a fix before Tuesday, then please grab this back. If you can wait a bit, I'll be happy to look at this then.

Alex pointed out to me that the whitespace bit is stored (in case of all 3: KURL / WebURL / GURL) in url::Parsed::whitespace_removed field from url/third_party/mozilla/url_parse.h

I see that for legacy IPC, ParamTraits<GURL>::Write only writes gurl.possibly_invalid_spec() (taking some precautions to avoid turning an invalid GURL into a valid GURL this way) and then ParamTraits<GURL>::Read passes this string to GURL's constructor.  This seems to loose information from url::Parsed.  Similar thing happens for mojo's StructTraits<..., GURL>.

Mike - would it be possible to look at blink::KURL::WhitespaceRemoved from somewhere in blink::FrameLoader?  Or is it too early / are we worried that this might miss some navigations?

+brettw@ in case this bug is somewhat/remotely related to the TODO in ParamTraits<GURL>::Write (*) [with possibly wrong bug id inside the TODO comment?] 

(*) https://cs.chromium.org/chromium/src/url/ipc/url_param_traits.cc?rcl=0e81c42f0794ac126f6414861e8632df8907d913&l=38

+japhet@ as an FYI regarding discussion about the best place where we should/can inspect presence of whitespace in an URL being navigated to.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94ad18d772d3d2fbf8c084b93af70502432a4256

commit 94ad18d772d3d2fbf8c084b93af70502432a4256
Author: nasko <nasko@chromium.org>
Date: Thu Apr 27 20:29:01 2017

Disable failing/crashing layout tests on Site Isolation bots.

These tests are failing consistently on the Site Isolation FYI bots.
Disabling them while they are investigated and fixed.

BUG= 710098 , 716085 

Review-Url: https://codereview.chromium.org/2845093004
Cr-Commit-Position: refs/heads/master@{#467767}

[modify] https://crrev.com/94ad18d772d3d2fbf8c084b93af70502432a4256/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b51a81e48c43becbb456ff0f0820abd950ac2834

commit b51a81e48c43becbb456ff0f0820abd950ac2834
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Thu Jul 20 17:46:53 2017

Re-enable --site-per-process layout tests that have healed themselves.

NOTRY=true

Bug:  710098 ,  623268 ,  623210 ,  680307 
Change-Id: I3b086fb6310ba8216cbade6cae6de08e689d460d
Reviewed-on: https://chromium-review.googlesource.com/579891
Reviewed-by: Lukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488295}
[modify] https://crrev.com/b51a81e48c43becbb456ff0f0820abd950ac2834/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

alexmos@: Can this be closed now?