This seems to indicate that the very temporary metadata entry is updated to point to the public file. This would also mean that if the user should erase the file it would cause the temporary metadata entry to become invalid.

It seems like a better, more robust solution without these side effects would be to leave the temporary metadata entry alone and instead create a new one on another, persistent namespace (maybe a new one).

Could this cause the share operation to fail?  If so, we must fix this.  If not, it may be OK.

The sharing operation does not fail. The main concern is the surprising effects on the post-share public file once the temporary page is deleted based on its own namespace expiration/deletion rules. I clarified the title a bit.

Note: I don't understand why this was assigned to me so leaving it unassigned.

We are hoping you will fix it :-)

We do not think this is a blocker for M-67, so this does not need to be fixed immediately.

Let's fix this instead by sharing a content:// URL, as opposed to stopping deleting it from the public directory.  Still not urgent for M67.

Instead of a content URL, we'll just share the URL instead of the file in this case to avoid user surprise.

Clarification:

Our planned fix for M-67 is to share the URL instead.
(https://chromium-review.googlesource.com/c/chromium/src/+/1036610)

For M-68, we plan to use the content:// URL instead of sharing to the public directory.

We should also make a function in the OfflinePageBridge to use the ClientPolicyController function for determining temporary pages instead of hardcoding them like we did for M-67

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

commit ddc9304e626b24b3dfef75ed53144a2d4fa571b9
Author: Pete Williamson <petewil@chromium.org>
Date: Wed May 02 00:32:04 2018

Share Temporary pages by URL instead of by copying to public directory.

To alleviate potential privacy concerns, share temporary pages by URL.
If we leave the code as is, the user might not realize that when they share
a temporary page that it has a side effect of temporarily putting the
page into the public download directory. Until we can address that,
we'll fix this by sharing the URL of the page (old behavior).

A future change will share the page by content:// uri, which will not
have this potential privacy flaw.


Bug:  831780 
Change-Id: I84d268a05628d3641e02f5b63daad7dd0ee38565
Reviewed-on: https://chromium-review.googlesource.com/1036610
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555250}
[modify] https://crrev.com/ddc9304e626b24b3dfef75ed53144a2d4fa571b9/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
[modify] https://crrev.com/ddc9304e626b24b3dfef75ed53144a2d4fa571b9/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtilsTest.java

To avoid potential privacy concerns with our feature, we'd like to merge this into the M-67 branch.  We plan to ship our feature in M-67 via a finch flag.

This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

has this been verified in canary?

Thanks for getting back to me!

The fix was not in this morning's canary.  It went in about 5pm Tuesday not sure when the canary syncs.

I'll try again with tomorrow's canary.

I did verify this in a clean build of tip of tree, since Canary does not have my change yet.

Verified in today's Canary.

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

commit cbd5e47617e4d2e14bf0baee24c107e1bf503143
Author: Pete Williamson <petewil@chromium.org>
Date: Fri May 04 20:45:38 2018

Share Temporary pages by URL instead of by copying to public directory.

To alleviate potential privacy concerns, share temporary pages by URL.
If we leave the code as is, the user might not realize that when they share
a temporary page that it has a side effect of temporarily putting the
page into the public download directory. Until we can address that,
we'll fix this by sharing the URL of the page (old behavior).

A future change will share the page by content:// uri, which will not
have this potential privacy flaw.


Bug:  831780 
Change-Id: I84d268a05628d3641e02f5b63daad7dd0ee38565
Reviewed-on: https://chromium-review.googlesource.com/1036610
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555250}(cherry picked from commit ddc9304e626b24b3dfef75ed53144a2d4fa571b9)
Reviewed-on: https://chromium-review.googlesource.com/1045185
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#488}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/cbd5e47617e4d2e14bf0baee24c107e1bf503143/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
[modify] https://crrev.com/cbd5e47617e4d2e14bf0baee24c107e1bf503143/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtilsTest.java

The first part has been fixed, we now use a URL for temporary pages.  The second part still needs to be done, we should use a content URI and share that instead of a URL.

The second part of this fix was landed as
https://chromium-review.googlesource.com/c/chromium/src/+/1081086