New issue
Advanced search Search tips

Issue 767519 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 769449



Sign in to add a comment

Android WebView duplicates the logic for granting renderer an access to read files when restoring navigation entries

Project Member Reported by lukasza@chromium.org, Sep 21 2017

Issue description

RestoreFromPickle in android_webview/browser/state_serializer.cc has the following TODO:

    // Set up the file access rights for the selected navigation entry.
    // TODO(joth): This is duplicated from chrome/.../session_restore.cc and
    // should be shared e.g. in  NavigationController.  http://crbug.com/68222 
    const int id = web_contents->GetRenderProcessHost()->GetID();
    const content::PageState& page_state =
        controller.GetLastCommittedEntry()->GetPageState();
    const std::vector<base::FilePath>& file_paths =
        page_state.GetReferencedFiles();
    for (std::vector<base::FilePath>::const_iterator file = file_paths.begin();
         file != file_paths.end(); ++file) {
      content::ChildProcessSecurityPolicy::GetInstance()->GrantReadFile(id,
                                                                        *file);
    }

Let me use open this bug to track this TODO.

 
Cc: creis@chromium.org
Charlie - can you please comment on the usage of |web_contents->GetRenderProcessHost()->GetID()| above?  Do |page_state.GetReferencedFiles()| apply only to the main frame's renderer process and never to files in another renderer process (e.g. in an OOPIF)?
Blocking: 769449

Comment 3 by boliu@chromium.org, Sep 27 2017

Components: Mobile>WebView
Labels: OS-Android
Owner: ----
that section in session_restore.cc has been deleted in https://codereview.chromium.org/745053002

I wonder if this code is still needed.

Comment 4 by creis@chromium.org, Sep 28 2017

Great-- I mentioned in that CL that the session_restore.cc code had become redundant, so maybe we can remove this copy of it.

Re comment #1: The list of referenced files is still something that needs to be updated for OOPIF support, since it's only tracked per page.  That will require some changes to the serialization format.  (I think there's some discussion of it in https://bugs.chromium.org/p/chromium/issues/detail?id=620261#c15.

Comment 5 by boliu@chromium.org, Sep 30 2017

Owner: boliu@chromium.org
Status: Assigned (was: Untriaged)
well, removing code is easy. let me do that :D
https://chromium-review.googlesource.com/c/chromium/src/+/693576
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 2 2017

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

commit 3bbeaeac4f9affd5fc6de68bae4f0c1e45765477
Author: Bo Liu <boliu@chromium.org>
Date: Mon Oct 02 19:01:14 2017

aw: Remove unneeded GrantReadFile file

This has become obsolete awhile ago. See bug for details.

Bug:  767519 
Change-Id: I5895335f1fff232cb99e6b4e993051e71945ef41
Reviewed-on: https://chromium-review.googlesource.com/693576
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505707}
[modify] https://crrev.com/3bbeaeac4f9affd5fc6de68bae4f0c1e45765477/android_webview/browser/state_serializer.cc

Comment 7 by boliu@chromium.org, Oct 2 2017

Status: Fixed (was: Assigned)

Sign in to add a comment