Increase test coverage for Content:// urls

Add test for verifying that iframe contents that are served from content urls cannot be accessed by parent page.

BUG=659492
Committed: https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870
Cr-Commit-Position: refs/heads/master@{#430093}

[sgurun-gerrit only, 2016-11-03 22:37:52]

Description was changed from

==========
Add test for verifying that iframe contents that are served from content urls
cannot be accessed by parent page.

BUG=659492
==========

to

==========
Add test for verifying that iframe contents that are served from content urls
cannot be accessed by parent page.

BUG=659492
==========

[sgurun-gerrit only, 2016-11-03 22:37:52]

sgurun@chromium.org changed reviewers:
+ nick@chromium.org, tedchoc@chromium.org

[sgurun-gerrit only, 2016-11-03 22:38:38]

On 2016/11/03 22:37:52, sgurun wrote:
> mailto:sgurun@chromium.org changed reviewers:
> + mailto:nick@chromium.org, mailto:tedchoc@chromium.org

ted: please do a full review
nick: are there any other tests we need to add?

[ncarter (slow), 2016-11-03 23:06:06]

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
File chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
(right):

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:118:
* Test that a content URL is *ALLOWED* to access an image provided by a content
URL.
Do we have a test that makes sure an http page can't do the same thing?

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:125:
+ "  document.title = 'success';"
Shouldn't this start out as something other than 'success'?

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:129:
+ "  img.src = '" + createContentUrl(resource) + "';";
Does the image actually load if you don't add it to the DOM?

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:140:
ensureResourceRequestCountInContentProviderNotLessThan(resource, 1);
Shouldn't there be two loads (one for the page, one for the image?)

[sgurun-gerrit only, 2016-11-04 00:47:22]

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
File chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
(right):

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:118:
* Test that a content URL is *ALLOWED* to access an image provided by a content
URL.
On 2016/11/03 23:06:06, ncarter wrote:
> Do we have a test that makes sure an http page can't do the same thing?

added

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:125:
+ "  document.title = 'success';"
On 2016/11/03 23:06:06, ncarter wrote:
> Shouldn't this start out as something other than 'success'?

good point, removed.

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:129:
+ "  img.src = '" + createContentUrl(resource) + "';";
On 2016/11/03 23:06:06, ncarter wrote:
> Does the image actually load if you don't add it to the DOM?
No, it does not.
added to the dom.

https://codereview.chromium.org/2476823002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:140:
ensureResourceRequestCountInContentProviderNotLessThan(resource, 1);
On 2016/11/03 23:06:06, ncarter wrote:
> Shouldn't there be two loads (one for the page, one for the image?)

opps, fixed the test.

[ncarter (slow), 2016-11-04 17:36:23]

lgtm

[Ted C, 2016-11-04 22:04:45]

https://codereview.chromium.org/2476823002/diff/60001/chrome/android/javatest...
File chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
(right):

https://codereview.chromium.org/2476823002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:88:
* Make sure a Content url *CANNOT* access to the contents of an iframe that is
loaded as a
s/access to the contents/access the contents

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
(right):

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java:87:
mResourceRequestCount.put(resource, mResourceRequestCount.get(resource));
why drop the "+ 1" here?

won't this always keep it as 1?

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java:210:
? mResourceRequestCount.get(resource)
should be indented 8 from the start of the previous line and the : 0 would fit
here too

[sgurun-gerrit only, 2016-11-04 22:40:45]

https://codereview.chromium.org/2476823002/diff/60001/chrome/android/javatest...
File chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
(right):

https://codereview.chromium.org/2476823002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:88:
* Make sure a Content url *CANNOT* access to the contents of an iframe that is
loaded as a
On 2016/11/04 22:04:45, Ted C wrote:
> s/access to the contents/access the contents

Done.

https://codereview.chromium.org/2476823002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java:158:
// Make sure image is not loaded by verifying that title is set to error.
removed stale comment.

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
(right):

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java:87:
mResourceRequestCount.put(resource, mResourceRequestCount.get(resource));
On 2016/11/04 22:04:45, Ted C wrote:
> why drop the "+ 1" here?
> 
> won't this always keep it as 1?

good catch. totally unintentional.    fortunately caught during review!!

This caused testContentUrlIframeAccessFromContentUrl to fail as the request
count now increased to 2. I fixed the test to check for "notlessthan 1" as the
other tests.

Now, you probably ask why the resource count is 2 and not 1. That is because in
our base/.../ContentUriUtils.java, the contentUriExists() calls content
resolver's openFileDescriptor() method to check if Uri exists. and later, to
read the resource, we call openFileDescriptor() again. This is probably why the
original tests always called notlessthan 1.

https://cs.chromium.org/chromium/src/base/android/java/src/org/chromium/base/...


Fixed.

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java:210:
? mResourceRequestCount.get(resource)
On 2016/11/04 22:04:45, Ted C wrote:
> should be indented 8 from the start of the previous line and the : 0 would fit
> here too

git cl format decided that. I can fix it if you like to. I moved it up but when
I run git cl format again, it came back :)

[Ted C, 2016-11-04 22:48:35]

lgtm

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
(right):

https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java:210:
? mResourceRequestCount.get(resource)
On 2016/11/04 22:40:45, sgurun wrote:
> On 2016/11/04 22:04:45, Ted C wrote:
> > should be indented 8 from the start of the previous line and the : 0 would
fit
> > here too
> 
> git cl format decided that. I can fix it if you like to. I moved it up but
when
> I run git cl format again, it came back :)

git cl format just doesn't work well on java sadly...i'd change it but I won't
force you to

[sgurun-gerrit only, 2016-11-04 22:53:11]

On 2016/11/04 22:48:35, Ted C wrote:
> lgtm
> 
>
https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
> File
>
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
> (right):
> 
>
https://codereview.chromium.org/2476823002/diff/60001/chrome/test/android/jav...
>
chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java:210:
> ? mResourceRequestCount.get(resource)
> On 2016/11/04 22:40:45, sgurun wrote:
> > On 2016/11/04 22:04:45, Ted C wrote:
> > > should be indented 8 from the start of the previous line and the : 0 would
> fit
> > > here too
> > 
> > git cl format decided that. I can fix it if you like to. I moved it up but
> when
> > I run git cl format again, it came back :)
> 
> git cl format just doesn't work well on java sadly...i'd change it but I won't
> force you to

fixed.

[sgurun-gerrit only, 2016-11-04 22:53:48]

The CQ bit was checked by sgurun@chromium.org

[sgurun-gerrit only, 2016-11-04 22:53:49]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org,
tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2476823002/#ps100001
(title: "Fixed indent")

[commit-bot: I haz the power, 2016-11-04 22:55:26]

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-11-05 00:38:28]

Description was changed from

==========
Add test for verifying that iframe contents that are served from content urls
cannot be accessed by parent page.

BUG=659492
==========

to

==========
Add test for verifying that iframe contents that are served from content urls
cannot be accessed by parent page.

BUG=659492
==========

[commit-bot: I haz the power, 2016-11-05 00:38:28]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-11-05 00:42:27]

Description was changed from

==========
Add test for verifying that iframe contents that are served from content urls
cannot be accessed by parent page.

BUG=659492
==========

to

==========
Add test for verifying that iframe contents that are served from content urls
cannot be accessed by parent page.

BUG=659492

Committed: https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870
Cr-Commit-Position: refs/heads/master@{#430093}
==========

[commit-bot: I haz the power, 2016-11-05 00:42:28]

Patchset 6 (id:??) landed as
https://crrev.com/1ddb80fd8f7bcba05f993b678d858ca687b98870
Cr-Commit-Position: refs/heads/master@{#430093}

[Mathieu, 2016-11-05 04:00:19]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2473153005/ by mathp@chromium.org.

The reason for reverting is: Failures in UrlSchemeTest

https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg....