Instr. test for calling onPageFinished as provisional JS redirect fails

Instr. test for calling onPageFinished as provisional JS redirect fails

BUG=557100

[gsennton, 2015-11-19 17:10:53]

gsennton@chromium.org changed reviewers:
+ mnaganov@chromium.org

[gsennton, 2015-11-19 17:10:53]

Hi Mikhail, this is my attempt at adding an instrumentation test for
crbug.com/557100 (together with lots of logging). This test does not behave as I
intend it to:

I would like to load a page containing an iframe and a JS redirect.
When receiving a request for the iframe the testserver would block until the JS
redirect fails (the redirect is meant to fail during its provisional phase) so
that the original page load does not finish until the JS redirect has failed.

What happens seems to be that we never reach the server-side code for the iframe
request and the original page finishes before the JS redirect fails (and the
final fails, declaring that the last url passed to onPageFinished was the JS
redirect url).

Any ideas on what I am doing wrong here? :)

[mnaganov (inactive), 2015-11-19 17:43:18]

On 2015/11/19 17:10:53, gsennton wrote:
> Hi Mikhail, this is my attempt at adding an instrumentation test for
> crbug.com/557100 (together with lots of logging). This test does not behave as
I
> intend it to:
> 
> I would like to load a page containing an iframe and a JS redirect.
> When receiving a request for the iframe the testserver would block until the
JS
> redirect fails (the redirect is meant to fail during its provisional phase) so
> that the original page load does not finish until the JS redirect has failed.
> 
> What happens seems to be that we never reach the server-side code for the
iframe
> request and the original page finishes before the JS redirect fails (and the
> final fails, declaring that the last url passed to onPageFinished was the JS
> redirect url).
> 
> Any ideas on what I am doing wrong here? :)

I don't really see why the browser would start loading the iframe, when it has
just instructed to do a redirect. The script put directly into the page source
will start executing as the page continues to load, so the browser may just not
get to the iframe tag.

Perhaps, what you need to do is to remove the <script> tag, wait until the
browser
starts to retrieve the iframe, block there -- so the page is still in "loading"
state, but the UI thread is alive, and then execute your JS redirect via
`evaluateJavaScript`?

But maybe it's not actually what you want to test :)

[gsennton, 2015-11-20 02:22:13]

On 2015/11/19 17:43:18, mnaganov wrote:
> On 2015/11/19 17:10:53, gsennton wrote:
> > Hi Mikhail, this is my attempt at adding an instrumentation test for
> > crbug.com/557100 (together with lots of logging). This test does not behave
as
> I
> > intend it to:
> > 
> > I would like to load a page containing an iframe and a JS redirect.
> > When receiving a request for the iframe the testserver would block until the
> JS
> > redirect fails (the redirect is meant to fail during its provisional phase)
so
> > that the original page load does not finish until the JS redirect has
failed.
> > 
> > What happens seems to be that we never reach the server-side code for the
> iframe
> > request and the original page finishes before the JS redirect fails (and the
> > final fails, declaring that the last url passed to onPageFinished was the JS
> > redirect url).
> > 
> > Any ideas on what I am doing wrong here? :)
> 
> I don't really see why the browser would start loading the iframe, when it has
> just instructed to do a redirect. The script put directly into the page source
> will start executing as the page continues to load, so the browser may just
not
> get to the iframe tag.
> 
> Perhaps, what you need to do is to remove the <script> tag, wait until the
> browser
> starts to retrieve the iframe, block there -- so the page is still in
"loading"
> state, but the UI thread is alive, and then execute your JS redirect via
> `evaluateJavaScript`?
> 
> But maybe it's not actually what you want to test :)

This helps (at least to make the original navigation wait for the redirect).

There is still the problem that we only post an onPageFinished for the JS
redirect (not the original load), I guess this is because the JS redirect is not
actually provisional but instead committed when it fails? (replacing the
original navigation?). Do you know how we can fail the redirect in its
provisional state?

[mnaganov (inactive), 2015-11-20 17:38:27]

On 2015/11/20 02:22:13, gsennton wrote:
> On 2015/11/19 17:43:18, mnaganov wrote:
> > On 2015/11/19 17:10:53, gsennton wrote:
> > > Hi Mikhail, this is my attempt at adding an instrumentation test for
> > > crbug.com/557100 (together with lots of logging). This test does not
behave
> as
> > I
> > > intend it to:
> > > 
> > > I would like to load a page containing an iframe and a JS redirect.
> > > When receiving a request for the iframe the testserver would block until
the
> > JS
> > > redirect fails (the redirect is meant to fail during its provisional
phase)
> so
> > > that the original page load does not finish until the JS redirect has
> failed.
> > > 
> > > What happens seems to be that we never reach the server-side code for the
> > iframe
> > > request and the original page finishes before the JS redirect fails (and
the
> > > final fails, declaring that the last url passed to onPageFinished was the
JS
> > > redirect url).
> > > 
> > > Any ideas on what I am doing wrong here? :)
> > 
> > I don't really see why the browser would start loading the iframe, when it
has
> > just instructed to do a redirect. The script put directly into the page
source
> > will start executing as the page continues to load, so the browser may just
> not
> > get to the iframe tag.
> > 
> > Perhaps, what you need to do is to remove the <script> tag, wait until the
> > browser
> > starts to retrieve the iframe, block there -- so the page is still in
> "loading"
> > state, but the UI thread is alive, and then execute your JS redirect via
> > `evaluateJavaScript`?
> > 
> > But maybe it's not actually what you want to test :)
> 
> This helps (at least to make the original navigation wait for the redirect).
> 
> There is still the problem that we only post an onPageFinished for the JS
> redirect (not the original load), I guess this is because the JS redirect is
not
> actually provisional but instead committed when it fails? (replacing the
> original navigation?). Do you know how we can fail the redirect in its
> provisional state?

You may look into this test:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/his...

What I have learned from it:

  -- there can be a client redirect on top of another pending client redirect
(RedirectTest.ClientCancelled);

  -- you can make a client redirect to a slowly loading page, so it will be a
"client redirect in a provisional state".

Perhaps you can initiate a client redirect, and then the server will first delay
a reply, and then return 404 -- that might make a JS redirect to be counted as
failed during a provisional load, perhaps. Or you might also simulate a click on
some link on the page while some parts of the page are still loading.

[gsennton, 2015-11-24 23:14:16]

Hey, so we now create a JS redirect when the server is about to respond to the
iframe request. We wait until the JS redirect reaches its provisional state and
then create a new navigation (which thus fails the JS redirect). The problem
with this is that we also finish the original load by creating a new navigation,
which means that we don't see the bug I'm trying to catch (where
shouldOverrideUrlLoading makes the JS redirect, but not the original page, fail
so that the JS redirect failure spawns an out-of order didStopLoad).

[mnaganov (inactive), 2015-11-25 18:18:24]

On 2015/11/24 23:14:16, gsennton wrote:
> Hey, so we now create a JS redirect when the server is about to respond to the
> iframe request. We wait until the JS redirect reaches its provisional state
and
> then create a new navigation (which thus fails the JS redirect). The problem
> with this is that we also finish the original load by creating a new
navigation,
> which means that we don't see the bug I'm trying to catch (where
> shouldOverrideUrlLoading makes the JS redirect, but not the original page,
fail
> so that the JS redirect failure spawns an out-of order didStopLoad).

I think I know how to fail the redirect in a provisional state. The trick is
that no single reply byte must be received from the server (otherwise, the load
will transition into "committed" state).

So what I tried was to throw a RuntimeException from the `redirectPath`
Runnable.
I had to modify the server to anticipate that:

In `TestWebServer.getResponse()`:

        try {
            if (response.mResponseAction != null)
response.mResponseAction.run();
        } catch (RuntimeException e) {
            throw new InterruptedException(e.toString());
        }

I removed the stuff related to loading of `newLoadUrl` from the `iframePath`
runnable (it's not needed anymore), and this is what I see in the log:

E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before load starturl
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after load starturl async
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before waiting for
onFailedLoadHelper callback
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageStarted with url
http://localhost:43487/startPath.html
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): In server! gonna wait
now! :D
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageStarted with url
http://localhost:43487/redirectPath.html
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): In server for redirect!
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageFinished with
url http://localhost:43487/redirectPath.html
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting for
onFailedLoadHelper callback
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before waiting for
onPageFinished callback
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): between waiting for
onPageFinished callback, onPageFinished url
http://localhost:43487/redirectPath.html
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting for
onPageFinished callback, onPageFinished url
http://localhost:43487/redirectPath.html
E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting third time
for for onPageFinished callback, onPageFinished url
http://localhost:43487/redirectPath.html

Is that what you wanted to achieve?

[gsennton, 2015-11-26 12:02:47]

On 2015/11/25 18:18:24, mnaganov wrote:
> On 2015/11/24 23:14:16, gsennton wrote:
> > Hey, so we now create a JS redirect when the server is about to respond to
the
> > iframe request. We wait until the JS redirect reaches its provisional state
> and
> > then create a new navigation (which thus fails the JS redirect). The problem
> > with this is that we also finish the original load by creating a new
> navigation,
> > which means that we don't see the bug I'm trying to catch (where
> > shouldOverrideUrlLoading makes the JS redirect, but not the original page,
> fail
> > so that the JS redirect failure spawns an out-of order didStopLoad).
> 
> I think I know how to fail the redirect in a provisional state. The trick is
> that no single reply byte must be received from the server (otherwise, the
load
> will transition into "committed" state).
> 
> So what I tried was to throw a RuntimeException from the `redirectPath`
> Runnable.
> I had to modify the server to anticipate that:
> 
> In `TestWebServer.getResponse()`:
> 
>         try {
>             if (response.mResponseAction != null)
> response.mResponseAction.run();
>         } catch (RuntimeException e) {
>             throw new InterruptedException(e.toString());
>         }
> 
> I removed the stuff related to loading of `newLoadUrl` from the `iframePath`
> runnable (it's not needed anymore), and this is what I see in the log:
> 
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before load starturl
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after load starturl
async
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before waiting for
> onFailedLoadHelper callback
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageStarted with
url
> http://localhost:43487/startPath.html
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): In server! gonna wait
> now! :D
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageStarted with
url
> http://localhost:43487/redirectPath.html
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): In server for redirect!
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageFinished with
> url http://localhost:43487/redirectPath.html
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting for
> onFailedLoadHelper callback
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before waiting for
> onPageFinished callback
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): between waiting for
> onPageFinished callback, onPageFinished url
> http://localhost:43487/redirectPath.html
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting for
> onPageFinished callback, onPageFinished url
> http://localhost:43487/redirectPath.html
> E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting third
time
> for for onPageFinished callback, onPageFinished url
> http://localhost:43487/redirectPath.html
> 
> Is that what you wanted to achieve?

The logs/onpagefinished checks are not really cleaned up I guess (and I
accidentally call super.onPageFinished from onPageStarted in the test....)

So, I assume you removed the jsRedirectSignal.await() call from the iframe
runnable (otherwise the redirect runnable is never run since the server is
single-threaded)?
If throwing an exception in the server and thus severing the connection for the
current request only makes the current request fail (not the startUrl/iframe)
then this approach should work if we can have the server be multi-threaded so
that it can serve the redirect and the iframe in parallel.

An observation: when the original load finishes before the redirect (didFinish
dispatched for startUrl and iframeUrl before didFailProvisional for redirectUrl)
even though the redirectUrl fails in a provisional state the didStopLoad is
called for the redirect url, which means that we in WebView won't dispatch
onPageFinished for the startUrl. I also see a didCommitProvisionalLoad call just
after the didFailProvisionalLoad (for redirectUrl) which seems strange...

[mnaganov (inactive), 2015-11-26 23:38:01]

On 2015/11/26 12:02:47, gsennton wrote:
> On 2015/11/25 18:18:24, mnaganov wrote:
> > On 2015/11/24 23:14:16, gsennton wrote:
> > > Hey, so we now create a JS redirect when the server is about to respond to
> the
> > > iframe request. We wait until the JS redirect reaches its provisional
state
> > and
> > > then create a new navigation (which thus fails the JS redirect). The
problem
> > > with this is that we also finish the original load by creating a new
> > navigation,
> > > which means that we don't see the bug I'm trying to catch (where
> > > shouldOverrideUrlLoading makes the JS redirect, but not the original page,
> > fail
> > > so that the JS redirect failure spawns an out-of order didStopLoad).
> > 
> > I think I know how to fail the redirect in a provisional state. The trick is
> > that no single reply byte must be received from the server (otherwise, the
> load
> > will transition into "committed" state).
> > 
> > So what I tried was to throw a RuntimeException from the `redirectPath`
> > Runnable.
> > I had to modify the server to anticipate that:
> > 
> > In `TestWebServer.getResponse()`:
> > 
> >         try {
> >             if (response.mResponseAction != null)
> > response.mResponseAction.run();
> >         } catch (RuntimeException e) {
> >             throw new InterruptedException(e.toString());
> >         }
> > 
> > I removed the stuff related to loading of `newLoadUrl` from the `iframePath`
> > runnable (it's not needed anymore), and this is what I see in the log:
> > 
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before load starturl
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after load starturl
> async
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before waiting for
> > onFailedLoadHelper callback
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageStarted with
> url
> > http://localhost:43487/startPath.html
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): In server! gonna wait
> > now! :D
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageStarted with
> url
> > http://localhost:43487/redirectPath.html
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): In server for
redirect!
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): in onPageFinished
with
> > url http://localhost:43487/redirectPath.html
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting for
> > onFailedLoadHelper callback
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): before waiting for
> > onPageFinished callback
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): between waiting for
> > onPageFinished callback, onPageFinished url
> > http://localhost:43487/redirectPath.html
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting for
> > onPageFinished callback, onPageFinished url
> > http://localhost:43487/redirectPath.html
> > E/AwContentsClientShouldOverrideUrlLoadingTest(32551): after waiting third
> time
> > for for onPageFinished callback, onPageFinished url
> > http://localhost:43487/redirectPath.html
> > 
> > Is that what you wanted to achieve?
> 
> The logs/onpagefinished checks are not really cleaned up I guess (and I
> accidentally call super.onPageFinished from onPageStarted in the test....)
> 
> So, I assume you removed the jsRedirectSignal.await() call from the iframe
> runnable (otherwise the redirect runnable is never run since the server is
> single-threaded)?
> If throwing an exception in the server and thus severing the connection for
the
> current request only makes the current request fail (not the startUrl/iframe)
> then this approach should work if we can have the server be multi-threaded so
> that it can serve the redirect and the iframe in parallel.
> 
> An observation: when the original load finishes before the redirect (didFinish
> dispatched for startUrl and iframeUrl before didFailProvisional for
redirectUrl)
> even though the redirectUrl fails in a provisional state the didStopLoad is
> called for the redirect url, which means that we in WebView won't dispatch
> onPageFinished for the startUrl. I also see a didCommitProvisionalLoad call
just
> after the didFailProvisionalLoad (for redirectUrl) which seems strange...

Ah, right -- you need to have a multi-threaded server if you need to keep the
main page in "loading" state while you are serving a redirect. Actually, you can
launch two instances of TestWebServer -- one for http and one for https, which
should result in having two server threads. I guess, redirecting from http to
https shouldn't be a problem.

[gsennton, 2015-11-30 13:17:35]

I think PS3 is the most reliable way to get this to work -- I just wait in
shouldOverrideUrlLoading for the redirect for a certain amount of time (so that
the load has a chance to enter its provisional state).

I'm not sure why throwing an exception in the test server does not seem to work
(AFAICT), it seems the redirect gets committed (and thus I can't reproduce the
bug).

[mnaganov (inactive), 2015-11-30 17:16:39]

OK. At least now it seems to fail reliably. Is it correct that in the case when
it passes, it doesn't actually need to wait for the sleep to complete?

https://codereview.chromium.org/1457343002/diff/40001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
(right):

https://codereview.chromium.org/1457343002/diff/40001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java:338:
public void onFailedLoad(String url) {}
Please rename it to 'onFailedLoadForTesting'.

https://codereview.chromium.org/1457343002/diff/40001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
(right):

https://codereview.chromium.org/1457343002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java:1160:
final String redirectPath = "/redirectPath.html";
It's better to use names like 'targetPath' and 'targetUrl', as it's the target
of a redirect. Otherwise it can be confused with a redirect source.

https://codereview.chromium.org/1457343002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java:1183:
final String redirectJs = "window.location.href='" + redirectUrl + "';";
...but this should remain 'redirectJs', as it's the source part of the redirect.

https://codereview.chromium.org/1457343002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java:1212:
onFailedLoadHelper.waitForCallback(0);
You should introduce onFailedLoadCallCount.

[gsennton, 2015-12-01 11:49:29]

>>> Is it correct that in the case when it passes, it doesn't actually need to
wait for the sleep to complete?

The sleeping is only to increase the probability of the redirect load reaching a
provisional state before we cancel it. Without the sleep the test is flaky, so
without the sleep we should pass the test with or without any fix. (the
provisional state seems to be reached long before the sleep is over though, so
it is probably possible to decrease the sleeping time, I haven't played around
with this).

I guess we want to add this test to the CL that fixes the issue so we don't
introduce a failing test if the CL needs to be reverted. ATM the latest version
of the fix is over in https://codereview.chromium.org/1481063002/, for now that
fix fails a browsertest (which was already flaky) and a layout test.

https://codereview.chromium.org/1457343002/diff/40001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
(right):

https://codereview.chromium.org/1457343002/diff/40001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java:338:
public void onFailedLoad(String url) {}
On 2015/11/30 17:16:39, mnaganov wrote:
> Please rename it to 'onFailedLoadForTesting'.

Done.

https://codereview.chromium.org/1457343002/diff/40001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
(right):

https://codereview.chromium.org/1457343002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java:1160:
final String redirectPath = "/redirectPath.html";
On 2015/11/30 17:16:39, mnaganov wrote:
> It's better to use names like 'targetPath' and 'targetUrl', as it's the target
> of a redirect. Otherwise it can be confused with a redirect source.

Done.

https://codereview.chromium.org/1457343002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java:1212:
onFailedLoadHelper.waitForCallback(0);
On 2015/11/30 17:16:39, mnaganov wrote:
> You should introduce onFailedLoadCallCount.

Done.

[mnaganov (inactive), 2015-12-01 16:42:22]

LGTM

Right, you shouldn't be committing a test that is currently failing. You can
either combine it with the patch that introduces the fix, or mark the test as
@DisabledTest for now, and then enable it in the fixing patch.

https://codereview.chromium.org/1457343002/diff/60001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
(right):

https://codereview.chromium.org/1457343002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java:337:
// Only used for testing
nit: After the rename, this comment is not needed anymore :)

[gsennton, 2015-12-01 16:55:20]

On 2015/12/01 16:42:22, mnaganov wrote:
> LGTM
> 
> Right, you shouldn't be committing a test that is currently failing. You can
> either combine it with the patch that introduces the fix, or mark the test as
> @DisabledTest for now, and then enable it in the fixing patch.
> 
>
https://codereview.chromium.org/1457343002/diff/60001/android_webview/java/sr...
> File
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
> (right):
> 
>
https://codereview.chromium.org/1457343002/diff/60001/android_webview/java/sr...
>
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java:337:
> // Only used for testing
> nit: After the rename, this comment is not needed anymore :)

Right, thanks :) I added the test to https://codereview.chromium.org/1440933004/
where the fix is. I will add you as reviewer there.

[mnaganov (inactive), 2015-12-01 16:57:20]

On 2015/12/01 16:55:20, gsennton wrote:
> On 2015/12/01 16:42:22, mnaganov wrote:
> > LGTM
> > 
> > Right, you shouldn't be committing a test that is currently failing. You can
> > either combine it with the patch that introduces the fix, or mark the test
as
> > @DisabledTest for now, and then enable it in the fixing patch.
> > 
> >
>
https://codereview.chromium.org/1457343002/diff/60001/android_webview/java/sr...
> > File
> android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/1457343002/diff/60001/android_webview/java/sr...
> >
>
android_webview/java/src/org/chromium/android_webview/AwContentsClient.java:337:
> > // Only used for testing
> > nit: After the rename, this comment is not needed anymore :)
> 
> Right, thanks :) I added the test to
https://codereview.chromium.org/1440933004/
> where the fix is. I will add you as reviewer there.

You can just TBR me on that CL then.

[gsennton, 2015-12-09 16:14:23]

Description was changed from

==========
Instr. test for calling onPageFinished as provisional JS redirect fails

BUG=557100
==========

to

==========
Instr. test for calling onPageFinished as provisional JS redirect fails

BUG=557100
==========