Ensured that worker thread renderer process file permissions are inherited from its main thread r...

blob_storage_controller.cc assert from this bug was caused by the fact that worker thread actually run in a different renderer process from the main page JS thread. chrome.fileBrowserPrivate.* methods grant access to files through ChildProcessSecurityPolicy class, but such file permissions would end up associated with renderer process of the main thread only. When worker tries to register blobs representing such files, ChildProcessSecurityPolicy check would reject it since its client process id has nothing to do with main renderer's id.

This CL adds mapping between worker and main renderer processes and uses that to ensure that worker thread renderer process file permissions are "inherited" from its main JS thread renderer child process.

BUG=chromium-os:14680
TEST=added extra tests to ChildProcessSecurityPolicyTest.FilePermissions
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=83754

[darin (slow to review), 2011-04-30 22:14:31]

Jian should also review this.

[jianli, 2011-05-01 07:12:20]

Not sure I fully understand this bug. Does this only happen in ChromeOS?

Adding dumi since he wrote the code for
ChildProcessSecurityPolicy::HasPermissionsForFile.

[zel, 2011-05-01 15:08:17]

This is happening anywhere where you have explicit file permissions given to
a renderer process. If such process kicks off a worker thread, such
permissions are not carried over there. On ChromeOS, we have an obvious use
case where this breaks in our file browser.


On Sun, May 1, 2011 at 12:12 AM, <jianli@chromium.org> wrote:

> Not sure I fully understand this bug. Does this only happen in ChromeOS?
>
> Adding dumi since he wrote the code for
> ChildProcessSecurityPolicy::HasPermissionsForFile.
>
>
> http://codereview.chromium.org/6893145/
>

[jianli, 2011-05-02 17:04:19]

John is looking into this. Removed dumi from the reviewer list.

[jam, 2011-05-02 17:17:53]

lgtm

Dmitry: FYI.  this change can be reverted when workers run in the renderers

http://codereview.chromium.org/6893145/diff/15/content/browser/child_process_...
File content/browser/child_process_security_policy.cc (right):

http://codereview.chromium.org/6893145/diff/15/content/browser/child_process_...
content/browser/child_process_security_policy.cc:168: main_render_process_id));
nit: seems simpler to read if it's just

worker_map_[child_id] = main_render_process_id;

http://codereview.chromium.org/6893145/diff/15/content/browser/child_process_...
content/browser/child_process_security_policy.cc:415: // let's check that its
main thread renderer process has access to that
nit "main thread" seems redundant.  "its renderer process" is enough

[zel, 2011-05-02 18:16:11]

http://codereview.chromium.org/6893145/diff/15/content/browser/child_process_...
File content/browser/child_process_security_policy.cc (right):

http://codereview.chromium.org/6893145/diff/15/content/browser/child_process_...
content/browser/child_process_security_policy.cc:168: main_render_process_id));
On 2011/05/02 17:17:53, John Abd-El-Malek wrote:
> nit: seems simpler to read if it's just
> 
> worker_map_[child_id] = main_render_process_id;

Done.

http://codereview.chromium.org/6893145/diff/15/content/browser/child_process_...
content/browser/child_process_security_policy.cc:415: // let's check that its
main thread renderer process has access to that
On 2011/05/02 17:17:53, John Abd-El-Malek wrote:
> nit "main thread" seems redundant.  "its renderer process" is enough

Done.