Do not drop the final web app scope path component if it ends with a slash.

Do not drop the final web app scope path component if it ends with a slash.

This CL updates the web app scope computation. The current computation
drops the final component of the path if there is more than one
component. For web apps which have a start URL that ends in a folder,
this removes the containing folder, resulting in an overly broad scope.
For instance, https://example.com/today/ would end up with a scope
over all of https://example.com under this scheme.

Instead, we refine the computation by not dropping the final path
component if the path ends with a slash. This indicates that the final
component is a directory which should be retained. Otherwise, a lack of
a slash at the end of the path signals a file, which we remove to
extract the containing directory.

BUG=688888
Review-Url: https://codereview.chromium.org/2711563003
Cr-Commit-Position: refs/heads/master@{#452279}
Committed: https://chromium.googlesource.com/chromium/src/+/4141e5db756f115ae51c31f71d33d6c3a8422638

[dominickn, 2017-02-22 03:48:19]

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

[dominickn, 2017-02-22 03:48:42]

Description was changed from

==========
Do not drop a web app scope path component if it ends with a slash.

This CL updates the web app scope computation. The current computation
drops the final component of the path if there is more than one
component. For web apps which have a start URL that ends in a folder,
this removes the containing folder, resulting in an overly broad scope.

Instead, we refine the computation by not dropping the final path
component if the path ends with a slash. This indicates that the final
component is a directory which should be retained. Otherwise, a lack of
a slash at the end of the path signals a file, which we remove to
extract the containing directory.

BUG=688888
==========

to

==========
Do not drop the final web app scope path component if it ends with a slash.

This CL updates the web app scope computation. The current computation
drops the final component of the path if there is more than one
component. For web apps which have a start URL that ends in a folder,
this removes the containing folder, resulting in an overly broad scope.

Instead, we refine the computation by not dropping the final path
component if the path ends with a slash. This indicates that the final
component is a directory which should be retained. Otherwise, a lack of
a slash at the end of the path signals a file, which we remove to
extract the containing directory.

BUG=688888
==========

[dominickn, 2017-02-22 03:48:42]

dominickn@chromium.org changed reviewers:
+ yfriedman@chromium.org

[dominickn, 2017-02-22 03:49:00]

dominickn@chromium.org changed reviewers:
+ pkotwicz@chromium.org

[dominickn, 2017-02-22 03:49:01]

PTAL, thanks!

[commit-bot: I haz the power, 2017-02-22 03:49:40]

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

[dominickn, 2017-02-22 03:49:46]

Description was changed from

==========
Do not drop the final web app scope path component if it ends with a slash.

This CL updates the web app scope computation. The current computation
drops the final component of the path if there is more than one
component. For web apps which have a start URL that ends in a folder,
this removes the containing folder, resulting in an overly broad scope.

Instead, we refine the computation by not dropping the final path
component if the path ends with a slash. This indicates that the final
component is a directory which should be retained. Otherwise, a lack of
a slash at the end of the path signals a file, which we remove to
extract the containing directory.

BUG=688888
==========

to

==========
Do not drop the final web app scope path component if it ends with a slash.

This CL updates the web app scope computation. The current computation
drops the final component of the path if there is more than one
component. For web apps which have a start URL that ends in a folder,
this removes the containing folder, resulting in an overly broad scope.
For instance, https://example.com/today/ would end up with a scope
over all of https://example.com under this scheme.

Instead, we refine the computation by not dropping the final path
component if the path ends with a slash. This indicates that the final
component is a directory which should be retained. Otherwise, a lack of
a slash at the end of the path signals a file, which we remove to
extract the containing directory.

BUG=688888
==========

[commit-bot: I haz the power, 2017-02-22 05:50:26]

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

[commit-bot: I haz the power, 2017-02-22 05:50:27]

Dry run: This issue passed the CQ dry run.

[Yaron, 2017-02-22 16:30:53]

https://codereview.chromium.org/2711563003/diff/1/chrome/android/junit/src/or...
File
chrome/android/junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java
(right):

https://codereview.chromium.org/2711563003/diff/1/chrome/android/junit/src/or...
chrome/android/junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java:38:
String url12 = "http://www.google.com:8000/maps/au/index.html?q=maps#fragment/";
AFAIK there's nothing with prevents a query params/fragment but to a dir:
e.g. "https://www.google.com/maps/au/north/?q=maps#fragment"

Please add a test

[Yaron, 2017-02-22 16:31:36]

On 2017/02/22 16:30:53, Yaron (limited availability) wrote:
>
https://codereview.chromium.org/2711563003/diff/1/chrome/android/junit/src/or...
> File
> chrome/android/junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java
> (right):
> 
>
https://codereview.chromium.org/2711563003/diff/1/chrome/android/junit/src/or...
>
chrome/android/junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java:38:
> String url12 =
"http://www.google.com:8000/maps/au/index.html?q=maps#fragment/";
> AFAIK there's nothing with prevents a query params/fragment but to a dir:
> e.g. "https://www.google.com/maps/au/north/?q=maps#fragment"
> 
> Please add a test

err lgtm if it's straightforward and you agree

[pkotwicz, 2017-02-22 20:26:55]

lgtm

[dominickn, 2017-02-22 22:52:58]

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

[dominickn, 2017-02-22 22:53:06]

Thanks!

https://codereview.chromium.org/2711563003/diff/1/chrome/android/junit/src/or...
File
chrome/android/junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java
(right):

https://codereview.chromium.org/2711563003/diff/1/chrome/android/junit/src/or...
chrome/android/junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java:38:
String url12 = "http://www.google.com:8000/maps/au/index.html?q=maps#fragment/";
On 2017/02/22 16:30:53, Yaron (limited availability) wrote:
> AFAIK there's nothing with prevents a query params/fragment but to a dir:
> e.g. "https://www.google.com/maps/au/north/?q=maps#fragment"
> 
> Please add a test

Good point, done.

[commit-bot: I haz the power, 2017-02-22 22:53:46]

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, 2017-02-22 23:40:04]

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

[commit-bot: I haz the power, 2017-02-22 23:40:05]

Dry run: This issue passed the CQ dry run.

[dominickn, 2017-02-22 23:40:40]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2017-02-22 23:40:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkotwicz@chromium.org, yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/2711563003/#ps20001
(title: "Extra test for dir + fragment and query")

[commit-bot: I haz the power, 2017-02-22 23:41:37]

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, 2017-02-22 23:49:13]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487806840381090,
"parent_rev": "d0984e8e9374ac9926dd9f41ad873c0c0abd90b3", "commit_rev":
"4141e5db756f115ae51c31f71d33d6c3a8422638"}

[commit-bot: I haz the power, 2017-02-22 23:49:55]

Description was changed from

==========
Do not drop the final web app scope path component if it ends with a slash.

This CL updates the web app scope computation. The current computation
drops the final component of the path if there is more than one
component. For web apps which have a start URL that ends in a folder,
this removes the containing folder, resulting in an overly broad scope.
For instance, https://example.com/today/ would end up with a scope
over all of https://example.com under this scheme.

Instead, we refine the computation by not dropping the final path
component if the path ends with a slash. This indicates that the final
component is a directory which should be retained. Otherwise, a lack of
a slash at the end of the path signals a file, which we remove to
extract the containing directory.

BUG=688888
==========

to

==========
Do not drop the final web app scope path component if it ends with a slash.

This CL updates the web app scope computation. The current computation
drops the final component of the path if there is more than one
component. For web apps which have a start URL that ends in a folder,
this removes the containing folder, resulting in an overly broad scope.
For instance, https://example.com/today/ would end up with a scope
over all of https://example.com under this scheme.

Instead, we refine the computation by not dropping the final path
component if the path ends with a slash. This indicates that the final
component is a directory which should be retained. Otherwise, a lack of
a slash at the end of the path signals a file, which we remove to
extract the containing directory.

BUG=688888

Review-Url: https://codereview.chromium.org/2711563003
Cr-Commit-Position: refs/heads/master@{#452279}
Committed:
https://chromium.googlesource.com/chromium/src/+/4141e5db756f115ae51c31f71d33...
==========

[commit-bot: I haz the power, 2017-02-22 23:49:56]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4141e5db756f115ae51c31f71d33...