Re-enable storage for Suborigins.

Re-enable storage for Suborigins.

Suborigins should, according to the spec, have access to their own
storage partitions. This CL re-enables the storage objects for
suborigins. Additionally, it implements the Cookies Tree Model updates
to make sure that the storage partitions appear under their parent
host's storage object.

BUG=608564, 336894
Committed: https://crrev.com/142972d9a3a304b276727f5167a06920587bf582
Cr-Commit-Position: refs/heads/master@{#423446}

[jww, 2016-05-23 23:23:50]

jww@chromium.org changed reviewers:
+ msramek@chromium.org

[jww, 2016-05-23 23:23:51]

Hi Martin. Would you mind taking a look at this CL, in particular the cookies
tree model bits? I've been talking to dmurph about it, but I'm happy to give any
context you need. Thanks!

[msramek, 2016-05-25 16:09:28]

msramek@chromium.org changed reviewers:
+ mkwst@chromium.org

[msramek, 2016-05-25 16:09:29]

Hi Joel!

As we discussed elsewhere: This LGTM in the sense that I agree the CL does what
is described. I'm adding Mike (TBR, since he's OOO) to comment on the
desirability, but if you have already talked to him, go forward :)

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/cookies_tree_model.cc (right):

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/cookies_tree_model.cc:79: // "google.com".
Please expand the comment with the treatment of suborigins.

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/cookies_tree_model_unittest.cc (right):

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/cookies_tree_model_unittest.cc:1687:
"http://host1:1/,http://host2:2/,http://foobar_host3:3/,http://"
nit: line break after the last comma rather than the scheme separator?

[jww, 2016-05-25 20:46:21]

jww@chromium.org changed reviewers:
+ nasko@chromium.org

[jww, 2016-05-25 20:46:22]

nasko@, would you mind reviewing the content/* changes? Thanks!

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/cookies_tree_model.cc (right):

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/cookies_tree_model.cc:79: // "google.com".
On 2016/05/25 16:09:28, msramek wrote:
> Please expand the comment with the treatment of suborigins.

Done.

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/cookies_tree_model_unittest.cc (right):

https://codereview.chromium.org/2005783005/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/cookies_tree_model_unittest.cc:1687:
"http://host1:1/,http://host2:2/,http://foobar_host3:3/,http://"
On 2016/05/25 16:09:29, msramek wrote:
> nit: line break after the last comma rather than the scheme separator?

Done.

[nasko, 2016-05-25 21:46:19]

I would like to understand what are the failures we will encounter with domains
in the wild that contain underscores before stamping this CL.

https://codereview.chromium.org/2005783005/diff/20001/content/common/origin_u...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/20001/content/common/origin_u...
content/common/origin_util.cc:90: return url.host().find("_") !=
std::string::npos;
This seems wrong. Underscores function just fine in DNS.
http://_test.netsekure.org/ works in Chrome just fine and this method will
return true.

It is true that this code matches the spec, but it looks like the spec needs
updating. Also, I'm not sure if we should be committing this code before
resolving this issue.

$ dig _test.netsekure.org
...
;; ANSWER SECTION:
_test.netsekure.org.    2806    IN      CNAME   netsekure.org.
netsekure.org.          2806    IN      A       96.47.74.180
...

$ dig us_test.netsekure.org
...
;; ANSWER SECTION:
us_test.netsekure.org.  2938    IN      CNAME   netsekure.org.
netsekure.org.          2802    IN      A       96.47.74.180
...

https://codereview.chromium.org/2005783005/diff/20001/content/public/common/o...
File content/public/common/origin_util.h (right):

https://codereview.chromium.org/2005783005/diff/20001/content/public/common/o...
content/public/common/origin_util.h:38: // If the host in the URL has a
Suborigin serialized into it, returns the name.
Follow the pattern of empty line between method and the next comment.

[jww, 2016-09-26 18:15:29]

nasko@, I think your major issue was addressed by
https://codereview.chromium.org/2332263002/. Can you take another look at this
CL? Thanks!

https://codereview.chromium.org/2005783005/diff/20001/content/common/origin_u...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/20001/content/common/origin_u...
content/common/origin_util.cc:90: return url.host().find("_") !=
std::string::npos;
On 2016/05/25 21:46:19, nasko (slow) wrote:
> This seems wrong. Underscores function just fine in DNS.
> http://_test.netsekure.org/ works in Chrome just fine and this method will
> return true.
> 
> It is true that this code matches the spec, but it looks like the spec needs
> updating. Also, I'm not sure if we should be committing this code before
> resolving this issue.
> 
> $ dig http://_test.netsekure.org
> ...
> ;; ANSWER SECTION:
> http://_test.netsekure.org.    2806    IN      CNAME   http://netsekure.org.
> http://netsekure.org.          2806    IN      A       96.47.74.180
> ...
> 
> $ dig http://us_test.netsekure.org
> ...
> ;; ANSWER SECTION:
> http://us_test.netsekure.org.  2938    IN      CNAME   http://netsekure.org.
> http://netsekure.org.          2802    IN      A       96.47.74.180
> ...

I believe this has been addressed by
https://codereview.chromium.org/2332263002/.

https://codereview.chromium.org/2005783005/diff/20001/content/public/common/o...
File content/public/common/origin_util.h (right):

https://codereview.chromium.org/2005783005/diff/20001/content/public/common/o...
content/public/common/origin_util.h:38: // If the host in the URL has a
Suborigin serialized into it, returns the name.
On 2016/05/25 21:46:19, nasko (slow) wrote:
> Follow the pattern of empty line between method and the next comment.

Done.

[jww, 2016-09-26 18:16:00]

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

[commit-bot: I haz the power, 2016-09-26 18:16:31]

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

[nasko, 2016-09-26 18:26:13]

https://codereview.chromium.org/2005783005/diff/60001/content/common/origin_u...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/60001/content/common/origin_u...
content/common/origin_util.cc:94: return url.host().find("_") !=
std::string::npos;
Huh, I thought we changed how we serialize suborigins. No?

[commit-bot: I haz the power, 2016-09-26 18:38:35]

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

[commit-bot: I haz the power, 2016-09-26 18:38:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jww, 2016-09-26 21:18:57]

Alright, should be updated to the new serialization. Sorry about that!

Also, FWIW, I think the longer term solution is to build this into the actual
origin object, but that's a larger project that I'm starting.

https://codereview.chromium.org/2005783005/diff/60001/content/common/origin_u...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/60001/content/common/origin_u...
content/common/origin_util.cc:94: return url.host().find("_") !=
std::string::npos;
On 2016/09/26 18:26:13, nasko (slow) wrote:
> Huh, I thought we changed how we serialize suborigins. No?

Indeed! I'm a moron. New patch should address this.

[jww, 2016-09-26 21:19:19]

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

[commit-bot: I haz the power, 2016-09-26 21:19:57]

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-09-26 22:14:48]

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

[commit-bot: I haz the power, 2016-09-26 22:14:49]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nasko, 2016-09-26 22:54:55]

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:94: return url.scheme() == "http-so" ||
url.scheme() == "https-so";
Shouldn't the GURL be valid before we start poking it?

Please use the defined constants instead of hardcoded strings.

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:104: std::string HostFromUrlStripSuborigin(const
GURL& url) {
Unused method?

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:113: std::string SchemeFromUrlStripSuborigin(const
GURL& url) {
Unused method?

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:133: replacements.SetSchemeStr("http");
Use constants instead of hardcoded strings.

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
File content/common/origin_util_unittest.cc (right):

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util_unittest.cc:52: GURL
suborigin_url("https-so://foobar.example.com");
Should "https-so://foo.com" succeed?

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util_unittest.cc:62: }
What about some negative test cases? "https-so://."?

https://codereview.chromium.org/2005783005/diff/80001/content/public/common/o...
File content/public/common/origin_util.h (right):

https://codereview.chromium.org/2005783005/diff/80001/content/public/common/o...
content/public/common/origin_util.h:48: GURL CONTENT_EXPORT
UrlStripSuborigin(const GURL& url);
nit: StripSuboriginFromUrl? It indicates that the method is an action (strip
suborigin) on an object (URL).

[jww, 2016-09-28 23:35:15]

Hi nasko. I've addressed your comments, and I've eliminated a few other issues.
I hope this is less of a disaster than the last few versions I uploaded :-/

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:94: return url.scheme() == "http-so" ||
url.scheme() == "https-so";
On 2016/09/26 22:54:54, nasko (slow) wrote:
> Shouldn't the GURL be valid before we start poking it?
> 
> Please use the defined constants instead of hardcoded strings.

Done.

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:104: std::string HostFromUrlStripSuborigin(const
GURL& url) {
On 2016/09/26 22:54:54, nasko (slow) wrote:
> Unused method?

Done.

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:113: std::string SchemeFromUrlStripSuborigin(const
GURL& url) {
On 2016/09/26 22:54:55, nasko (slow) wrote:
> Unused method?

Done.

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util.cc:133: replacements.SetSchemeStr("http");
On 2016/09/26 22:54:54, nasko (slow) wrote:
> Use constants instead of hardcoded strings.

Done.

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
File content/common/origin_util_unittest.cc (right):

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util_unittest.cc:52: GURL
suborigin_url("https-so://foobar.example.com");
On 2016/09/26 22:54:55, nasko (slow) wrote:
> Should "https-so://foo.com" succeed?

Yes, it should. These are meant to be pure parsing mechanisms, effectively. As
long as its a valid URL, it will extract what would be the suborigin from it.

https://codereview.chromium.org/2005783005/diff/80001/content/common/origin_u...
content/common/origin_util_unittest.cc:62: }
On 2016/09/26 22:54:55, nasko (slow) wrote:
> What about some negative test cases? "https-so://."?

Done.

https://codereview.chromium.org/2005783005/diff/80001/content/public/common/o...
File content/public/common/origin_util.h (right):

https://codereview.chromium.org/2005783005/diff/80001/content/public/common/o...
content/public/common/origin_util.h:48: GURL CONTENT_EXPORT
UrlStripSuborigin(const GURL& url);
On 2016/09/26 22:54:55, nasko (slow) wrote:
> nit: StripSuboriginFromUrl? It indicates that the method is an action (strip
> suborigin) on an object (URL).

Done.

[nasko, 2016-09-29 16:49:56]

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:104: std::string host = url.host();
minor nit: I don't know how perf sensitive this call will be, but in general we
could probably save making a copy of the host portion by using StringPiece.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:106: // This means that there is no suborigin
before the dot, so there is no
Technically it means there is no dot at all : ).

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:108: if (host.find('.') == 0)
s/0/string::npos/?

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:113: if (host.find('.') == (host.size() - 1))
You could store the result of find from above and reuse it, avoiding doing the
string traversal twice.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:124: return url.host().substr(0, suborigin_end);
Check suborigin_end for equality of string::npos and avoid calling substr if
equal.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:139: size_t suborigin_end = url.host().find(".");
Again, check for string::npos.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
File content/common/origin_util_unittest.cc (right):

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util_unittest.cc:54: GURL
suborigin_bigger_url("https-so://foobar.example.com/some/path");
Why not throw in all the parts of an URL to ensure we don't drop them? Port,
query string, fragment :).

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util_unittest.cc:78: GURL
suborigin_but_empty_host_url("https-so://foo.");
What about testing StripSuboriginFromUrl and SuboriginFromUrl for those URLs?

https://codereview.chromium.org/2005783005/diff/140001/content/public/common/...
File content/public/common/origin_util.h (right):

https://codereview.chromium.org/2005783005/diff/140001/content/public/common/...
content/public/common/origin_util.h:44: // suborigin stripped. Otherwise,
returns the empty string.
Should it return an empty string or just the original URL? If returning an empty
string, then callers will have to explicitly check for empty and use the
original URL. If the method returns the original URL, it can just be called and
proceeded without checks. I'm not sure which one is better and it will likely
depend on the callers patterns. 

Also, it returns a GURL type, so it can't be empty string. 

Oh, it actually returns the original URL based on the current implementation, so
maybe just update the comment?

[jww, 2016-09-29 22:21:00]

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:104: std::string host = url.host();
On 2016/09/29 16:49:55, nasko (slow) wrote:
> minor nit: I don't know how perf sensitive this call will be, but in general
we
> could probably save making a copy of the host portion by using StringPiece.

Done.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:106: // This means that there is no suborigin
before the dot, so there is no
On 2016/09/29 16:49:56, nasko (slow) wrote:
> Technically it means there is no dot at all : ).

I don't understand. Are you referring to the fact that . is ignored in hostnames
if it's the first character? That's true for hostnames, but the suborigin spec
is pretty clear that this isn't possible in a suborigin serialization, unless
you were to have an empty-string suborigin, but that's disallowed separately.
So, basically, since we already know the scheme is a suborigin scheme at this
point, we can safely say that the "hostname" (which is really a serialization of
the suborigin + hostname) shouldn't start with a period.

By way of example, this is for the case (which I have in the unit test) of:
https-so://.foo.com

Here it's the empty string before the first period, which is an invalid
suborigin.

Ah, but I am missing the case where there is no period at all, so I'll add that.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:108: if (host.find('.') == 0)
On 2016/09/29 16:49:56, nasko (slow) wrote:
> s/0/string::npos/?

See above response. I did intend for this to check if the first character of the
hostname is a period, but perhaps I'm misunderstanding your comment.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:113: if (host.find('.') == (host.size() - 1))
On 2016/09/29 16:49:56, nasko (slow) wrote:
> You could store the result of find from above and reuse it, avoiding doing the
> string traversal twice.

Done.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:124: return url.host().substr(0, suborigin_end);
On 2016/09/29 16:49:56, nasko (slow) wrote:
> Check suborigin_end for equality of string::npos and avoid calling substr if
> equal. 

Done.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:139: size_t suborigin_end = url.host().find(".");
On 2016/09/29 16:49:56, nasko (slow) wrote:
> Again, check for string::npos.

Done.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
File content/common/origin_util_unittest.cc (right):

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util_unittest.cc:54: GURL
suborigin_bigger_url("https-so://foobar.example.com/some/path");
On 2016/09/29 16:49:56, nasko (slow) wrote:
> Why not throw in all the parts of an URL to ensure we don't drop them? Port,
> query string, fragment :).

Done.

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util_unittest.cc:78: GURL
suborigin_but_empty_host_url("https-so://foo.");
On 2016/09/29 16:49:56, nasko (slow) wrote:
> What about testing StripSuboriginFromUrl and SuboriginFromUrl for those URLs?

Done.

https://codereview.chromium.org/2005783005/diff/140001/content/public/common/...
File content/public/common/origin_util.h (right):

https://codereview.chromium.org/2005783005/diff/140001/content/public/common/...
content/public/common/origin_util.h:44: // suborigin stripped. Otherwise,
returns the empty string.
On 2016/09/29 16:49:56, nasko (slow) wrote:
> Should it return an empty string or just the original URL? If returning an
empty
> string, then callers will have to explicitly check for empty and use the
> original URL. If the method returns the original URL, it can just be called
and
> proceeded without checks. I'm not sure which one is better and it will likely
> depend on the callers patterns. 
> 
> Also, it returns a GURL type, so it can't be empty string. 
> 
> Oh, it actually returns the original URL based on the current implementation,
so
> maybe just update the comment?

Done.

[nasko, 2016-09-30 17:40:07]

Thanks for sticking through all my comments!

content/ LGTM

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/140001/content/common/origin_...
content/common/origin_util.cc:106: // This means that there is no suborigin
before the dot, so there is no
On 2016/09/29 22:21:00, jww wrote:
> On 2016/09/29 16:49:56, nasko (slow) wrote:
> > Technically it means there is no dot at all : ).
> 
> I don't understand. Are you referring to the fact that . is ignored in
hostnames
> if it's the first character? That's true for hostnames, but the suborigin spec
> is pretty clear that this isn't possible in a suborigin serialization, unless
> you were to have an empty-string suborigin, but that's disallowed separately.
> So, basically, since we already know the scheme is a suborigin scheme at this
> point, we can safely say that the "hostname" (which is really a serialization
of
> the suborigin + hostname) shouldn't start with a period.
> 
> By way of example, this is for the case (which I have in the unit test) of:
> https-so://.foo.com
> 
> Here it's the empty string before the first period, which is an invalid
> suborigin.
> 
> Ah, but I am missing the case where there is no period at all, so I'll add
that.

I read the code with the interpretation that it is looking for no dot. Thanks
for the info on the extra case you are checking for!

https://codereview.chromium.org/2005783005/diff/160001/content/common/origin_...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/160001/content/common/origin_...
content/common/origin_util.cc:112: if (first_period == 0 || first_period ==
std::string::npos)
I think std::string::npos and base::StringPiece::npos are equivalent, but since
you moved over to use StringPiece, it might be safer to check its version of
npos to be on the safe side.

[jww, 2016-09-30 17:52:17]

https://codereview.chromium.org/2005783005/diff/160001/content/common/origin_...
File content/common/origin_util.cc (right):

https://codereview.chromium.org/2005783005/diff/160001/content/common/origin_...
content/common/origin_util.cc:112: if (first_period == 0 || first_period ==
std::string::npos)
On 2016/09/30 17:40:07, nasko (slow) wrote:
> I think std::string::npos and base::StringPiece::npos are equivalent, but
since
> you moved over to use StringPiece, it might be safer to check its version of
> npos to be on the safe side.

Ah, didn't even know there was a StringPiece specific npos, thanks!

[jww, 2016-09-30 17:52:22]

The CQ bit was checked by jww@chromium.org

[jww, 2016-09-30 17:52:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2005783005/#ps200001
(title: "Rebase on ToT")

[commit-bot: I haz the power, 2016-09-30 17:53:08]

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-09-30 18:05:00]

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

[commit-bot: I haz the power, 2016-09-30 18:05:01]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jww, 2016-10-01 04:58:35]

mkwst@, can you review 
third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h? It's a one line
change in there, so if it's OK by you, feel free to throw on the commit queue.
Thanks!

[jww, 2016-10-01 04:58:52]

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

[commit-bot: I haz the power, 2016-10-01 04:59: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-10-01 05:46:46]

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

[commit-bot: I haz the power, 2016-10-01 05:46:47]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jww, 2016-10-05 02:31:35]

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

[commit-bot: I haz the power, 2016-10-05 02:32:10]

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-10-05 03:11:21]

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

[commit-bot: I haz the power, 2016-10-05 03:11:22]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Mike West, 2016-10-05 04:05:54]

LGTM, though the BrowsingDataLocalStorageHelperTest.DeleteSingleFile failure
looks relevant.

[jww, 2016-10-05 15:02:24]

On 2016/10/05 04:05:54, Mike West wrote:
> LGTM, though the BrowsingDataLocalStorageHelperTest.DeleteSingleFile failure
> looks relevant.

Indeed! That browser test is fixed in the latest patchset. Thanks!

[jww, 2016-10-05 15:02:31]

The CQ bit was checked by jww@chromium.org

[jww, 2016-10-05 15:02:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, nasko@chromium.org, mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2005783005/#ps240001
(title: "Fix for browser test")

[commit-bot: I haz the power, 2016-10-05 15:02:54]

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-10-05 18:38:36]

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

[commit-bot: I haz the power, 2016-10-05 18:38:38]

Failed to apply patch for
third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h:171
  Falling back to three-way merge...
  Applied patch to
'third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h' with conflicts.
  U third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h

Patch:       third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h
Index: third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h
diff --git a/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h
b/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h
index
ae595edc9149ca8a35cf802aa441ce06f43c8ab0..895233001f2981931da1c32aa2e66b96c902f53f
100644
--- a/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h
+++ b/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.h
@@ -171,7 +171,7 @@ public:
     bool isGrantedUniversalAccess() const { return m_universalAccess; }
 
     bool canAccessDatabase() const { return !isUnique(); }
-    bool canAccessLocalStorage() const { return !isUnique() && !hasSuborigin();
}
+    bool canAccessLocalStorage() const { return !isUnique(); }
     bool canAccessSharedWorkers() const { return !isUnique(); }
     bool canAccessServiceWorkers() const { return !isUnique() &&
!hasSuborigin(); }
     bool canAccessCookies() const { return !isUnique(); }

[jww, 2016-10-05 18:42:41]

The CQ bit was checked by jww@chromium.org

[jww, 2016-10-05 18:42:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, mkwst@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2005783005/#ps260001
(title: "Rebase on ToT")

[commit-bot: I haz the power, 2016-10-05 18:43:23]

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-10-05 22:59:42]

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

[commit-bot: I haz the power, 2016-10-05 22:59:43]

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_...)

[jww, 2016-10-06 01:51:58]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2016-10-06 01:52:33]

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-10-06 06:00:06]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-10-06 06:01:23]

Description was changed from

==========
Re-enable storage for Suborigins.

Suborigins should, according to the spec, have access to their own
storage partitions. This CL re-enables the storage objects for
suborigins. Additionally, it implements the Cookies Tree Model updates
to make sure that the storage partitions appear under their parent
host's storage object.

BUG=608564, 336894
==========

to

==========
Re-enable storage for Suborigins.

Suborigins should, according to the spec, have access to their own
storage partitions. This CL re-enables the storage objects for
suborigins. Additionally, it implements the Cookies Tree Model updates
to make sure that the storage partitions appear under their parent
host's storage object.

BUG=608564, 336894

Committed: https://crrev.com/142972d9a3a304b276727f5167a06920587bf582
Cr-Commit-Position: refs/heads/master@{#423446}
==========

[commit-bot: I haz the power, 2016-10-06 06:01:24]

Patchset 14 (id:??) landed as
https://crrev.com/142972d9a3a304b276727f5167a06920587bf582
Cr-Commit-Position: refs/heads/master@{#423446}