Fix the bug that webview guest cannot load script of a SharedWorker.

This CL aims to fix the bug that when webview guest create a SharedWorker, the sctipt file of SharedWorker can not be loaded. 

The SharedWorker lives in a separate render process as the webview who creates it. Because  url_request_util::AllowCrossRendererResourceLoad（...） doesn't consider that the render process of the SharedWorker  is a guest, so it denies the load of SharedWorker's script.

So we introduce webview partition id map in ExtensitonRenderState. When loading a SharedWorker script, we will check whether the script file is in the accessible resource of given "Partition ID", and decide whether it can be loaded or not. 

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278377

[Fady Samuel, 2014-06-16 21:01:57]

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.cc:258: void
ExtensionRendererState::AddWebViewPartitionID(
Remove this method.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.cc:262:
webview_partition_id_map[guest_process_id] = webview_partition_id;
Move this to AddWebView.

You probably also need a counter here. The counter needs to keep track of the
number of webviews are in a process. You don't always want to remove an item
from this map.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.cc:267:
webview_partition_id_map.erase(guest_process_id);
Move this to RemoveWebView.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_renderer_state.h (right):

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.h:46: // process.
Please mention that this should only be called on the IO thread

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.h:90: WebViewPartitionIDMap
webview_partition_id_map;
webview_partition_id_map_

https://codereview.chromium.org/333313004/diff/1/chrome/browser/guest_view/we...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/333313004/diff/1/chrome/browser/guest_view/we...
chrome/browser/guest_view/web_view/web_view_guest.cc:1131:
base::Bind(&ExtensionRendererState::AddWebViewPartitionID,
Now that I think about it, you porbably don't need AddWebViewPartitionID or
RemoveWebViewPartitionID. You can add/remove the information you need to a
secondary map in AddWebView and RemoveWebView!

https://codereview.chromium.org/333313004/diff/1/chrome/browser/guest_view/we...
chrome/browser/guest_view/web_view/web_view_guest.cc:1148:
content::BrowserThread::PostTask(
See the comment above.

[Xi Han, 2014-06-16 21:53:56]

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.cc:258: void
ExtensionRendererState::AddWebViewPartitionID(
On 2014/06/16 21:01:56, Fady Samuel wrote:
> Remove this method.

Done.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.cc:262:
webview_partition_id_map[guest_process_id] = webview_partition_id;
On 2014/06/16 21:01:56, Fady Samuel wrote:
> Move this to AddWebView.
> 
> You probably also need a counter here. The counter needs to keep track of the
> number of webviews are in a process. You don't always want to remove an item
> from this map.

Done.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.cc:267:
webview_partition_id_map.erase(guest_process_id);
On 2014/06/16 21:01:56, Fady Samuel wrote:
> Move this to RemoveWebView.

Done.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_renderer_state.h (right):

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.h:46: // process.
On 2014/06/16 21:01:56, Fady Samuel wrote:
> Please mention that this should only be called on the IO thread

Done.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_renderer_state.h:90: WebViewPartitionIDMap
webview_partition_id_map;
On 2014/06/16 21:01:56, Fady Samuel wrote:
> webview_partition_id_map_

Done.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/guest_view/we...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/333313004/diff/1/chrome/browser/guest_view/we...
chrome/browser/guest_view/web_view/web_view_guest.cc:1131:
base::Bind(&ExtensionRendererState::AddWebViewPartitionID,
On 2014/06/16 21:01:56, Fady Samuel wrote:
> Now that I think about it, you porbably don't need AddWebViewPartitionID or
> RemoveWebViewPartitionID. You can add/remove the information you need to a
> secondary map in AddWebView and RemoveWebView!

Done.

https://codereview.chromium.org/333313004/diff/1/chrome/browser/guest_view/we...
chrome/browser/guest_view/web_view/web_view_guest.cc:1148:
content::BrowserThread::PostTask(
On 2014/06/16 21:01:56, Fady Samuel wrote:
> See the comment above.

Done.

[Fady Samuel, 2014-06-16 22:02:32]

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:241:
partition_id_pair.second += 1;
++ instead of +=

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:242:
webview_partition_id_map_[guest_process_id] = partition_id_pair;
If you grab iter->second as a reference, then you won't need to do reassignment.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:257: PartitionID
partition_id_pair = iter->second;
Grab it as a reference?

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:258:
partition_id_pair.second -= 1;
-- instead of -= 1?

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:259:
webview_partition_id_map_[guest_process_id] = partition_id_pair;
If you grab it as a reference, you don't need to do this assignment.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.h (right):

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.h:68: typedef
std::pair<std::string, int> PartitionID;
Pairs are less readable. I think it would be preferable to do a struct.

struct WebViewPartitionInfo {
  int web_view_count;
  std::string partition_id;
};

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.h:82: const std::string&
partition_id,
Unnecessary.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/guest_vie...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/guest_vie...
chrome/browser/guest_view/web_view/web_view_guest.cc:1126: partition_id,
This extra parameter is unnecessary. You can grab the partition ID from
webview_info.partition_id.

While you're at it, for consistency, please rename the variable above from
webview_info to web_view_info.

[Xi Han, 2014-06-17 13:42:31]

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:241:
partition_id_pair.second += 1;
On 2014/06/16 22:02:32, Fady Samuel wrote:
> ++ instead of +=

Done.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:242:
webview_partition_id_map_[guest_process_id] = partition_id_pair;
On 2014/06/16 22:02:32, Fady Samuel wrote:
> If you grab iter->second as a reference, then you won't need to do
reassignment.
> 

Done.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:257: PartitionID
partition_id_pair = iter->second;
On 2014/06/16 22:02:32, Fady Samuel wrote:
> Grab it as a reference?

Done.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:258:
partition_id_pair.second -= 1;
On 2014/06/16 22:02:32, Fady Samuel wrote:
> -- instead of -= 1?

Done.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:259:
webview_partition_id_map_[guest_process_id] = partition_id_pair;
On 2014/06/16 22:02:32, Fady Samuel wrote:
> If you grab it as a reference, you don't need to do this assignment.

Done.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.h (right):

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.h:68: typedef
std::pair<std::string, int> PartitionID;
On 2014/06/16 22:02:32, Fady Samuel wrote:
> Pairs are less readable. I think it would be preferable to do a struct.
> 
> struct WebViewPartitionInfo {
>   int web_view_count;
>   std::string partition_id;
> };

Good idea, thanks.

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.h:82: const std::string&
partition_id,
On 2014/06/16 22:02:32, Fady Samuel wrote:
> Unnecessary.

Good catch!

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/guest_vie...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/333313004/diff/20001/chrome/browser/guest_vie...
chrome/browser/guest_view/web_view/web_view_guest.cc:1126: partition_id,
On 2014/06/16 22:02:32, Fady Samuel wrote:
> This extra parameter is unnecessary. You can grab the partition ID from
> webview_info.partition_id.
> 
> While you're at it, for consistency, please rename the variable above from
> webview_info to web_view_info.

Done.

[Fady Samuel, 2014-06-17 18:20:29]

https://codereview.chromium.org/333313004/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:243: WebViewPartitionInfo
partition_info ={1, webview_info.partition_id};
I can't find anything in the style guide that explicitly disallows this but
constructors are the more common pattern here. Please switch to a constructor.

[Xi Han, 2014-06-17 19:01:07]

https://codereview.chromium.org/333313004/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:243: WebViewPartitionInfo
partition_info ={1, webview_info.partition_id};
On 2014/06/17 18:20:29, Fady Samuel wrote:
> I can't find anything in the style guide that explicitly disallows this but
> constructors are the more common pattern here. Please switch to a constructor.

Done.

[Fady Samuel, 2014-06-18 13:39:16]

lgtm with nits. Please add Ben Kalman as a reviewer after you've addressed these
comments.

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:239: WebViewPartitionInfo&
partition_info = iter->second;
There's actually no need for this local variable:

--iter->second.web_view_count;

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:244:
webview_partition_id_map_.insert(
nit

This is more readable:

webview_partition_id_map_[guest_process_id] = partition_info;

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:259:
--partition_info.web_view_count;
There's no need for a local variable:

--iter->second.web_view_count;

[Xi Han, 2014-06-18 14:09:19]

Fady: small changes are made according to your comments, please take another
look.

Benjamin: would you please review this CL? Thanks a lot!

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
File chrome/browser/extensions/extension_renderer_state.cc (right):

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:239: WebViewPartitionInfo&
partition_info = iter->second;
On 2014/06/18 13:39:16, Fady Samuel wrote:
> There's actually no need for this local variable:
> 
> --iter->second.web_view_count;

Done.

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:244:
webview_partition_id_map_.insert(
On 2014/06/18 13:39:16, Fady Samuel wrote:
> nit
> 
> This is more readable:
> 
> webview_partition_id_map_[guest_process_id] = partition_info;

Done.

https://codereview.chromium.org/333313004/diff/60001/chrome/browser/extension...
chrome/browser/extensions/extension_renderer_state.cc:259:
--partition_info.web_view_count;
On 2014/06/18 13:39:16, Fady Samuel wrote:
> There's no need for a local variable:
> 
> --iter->second.web_view_count;

Done.

[not at google - send to devlin, 2014-06-18 22:28:28]

rubberstamp lgtm

[Xi Han, 2014-06-18 23:35:21]

The CQ bit was checked by hanxi@chromium.org

[commit-bot: I haz the power, 2014-06-18 23:37:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hanxi@chromium.org/333313004/80001

[commit-bot: I haz the power, 2014-06-19 15:13:05]

Change committed as 278377