Implement a prototype to render cross-site iframes in a separate process from their parent.

Implement a prototype to render cross-site iframes in a separate process from their parent.

This is a very early prototype and should not be used for general browsing of the web. To enable it, run Chrome with --site-per-process and --enable-browser-plugin-for-all-view-types command line parameters.

(initial implementation by irobert@chromium.org)

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

[nasko, 2013-01-16 21:50:07]

Hey guys,
The prototype for out-of-process iframes is ready for initial review.
Thanks,
Nasko

[Charlie Reis, 2013-01-17 19:44:54]

I think you're already aware of the TaskManager issues, but I put comments in
anyway.

Also, can you add a comment to the CL description that this is not yet safe for
general web use, since it exposes the webview API?  (Thankfully we already get
the scary infobar from the --enable-browser-plugin-for-all-view-types flag.)

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
File chrome/browser/task_manager/task_manager_resource_providers.cc (right):

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1853: // Given a
RenderViewHost, return true if this RVH is Cross-site Iframe.
This comment belongs in .h file.

We should have a separate comment within this function saying why the heuristic
it's using (all processes that don't have TabContents) is correct.

Except... it's not correct.  There are other RenderProcessHosts that don't have
TabContents, like background pages and notifications.  We'll need a different
approach.

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1855: if
(host==NULL || !host->GetHandle())
nit: !host

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1859: if
((*iterator)->GetRenderProcessHost()->GetHandle() == host->GetHandle())
Is it possible for this to crash if a TabContents doesn't have a RPH?

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (left):

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:156: 
No need for this change.

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:83: // same site
iframes in the same process when running in --site-per-process
Can you change this comment a bit?  We're not technically hitting this path for
same-site iframes (thanks to your change in ResourceLoader), so this is more
about putting the cross-site URLs into their respective SiteInstances instead of
a special guest SiteInstance for BrowserPlugins.

Maybe we should start by saying that --site-per-process temporarily breaks
BrowserPlugins and makes them behave like OOP iframes, with a TODO to fix that
by splitting up BrowserPlugin?

https://codereview.chromium.org/11772005/diff/20003/content/browser/child_pro...
File content/browser/child_process_security_policy_impl.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/child_pro...
content/browser/child_process_security_policy_impl.cc:512: // out-of-process
iframes is ready to go.
Wasn't this from your other CL?  Did you want to land that first?

https://codereview.chromium.org/11772005/diff/20003/content/browser/loader/re...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/loader/re...
content/browser/loader/resource_loader.cc:480: // Change the MIME type of the
response if --site-per-process flag
We should introduce this comment very clearly saying that it's a temporary
prototype for out-of-process iframes, and explain briefly how it works.  The
simple comment was fine when it was just for us, but others are going to wonder
what it's for if we land it.

https://codereview.chromium.org/11772005/diff/20003/content/browser/loader/re...
content/browser/loader/resource_loader.cc:481: // is passed and the current
request is for the Sub-Frame page. Avoid
nit: the Sub-Frame page -> for a subframe.

https://codereview.chromium.org/11772005/diff/20003/content/browser/plugin_se...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/plugin_se...
content/browser/plugin_service_impl.cc:150: // Register MIME type to support
WebView component.
This needs to be behind the --site-per-process flag.  Don't want this plugin
available on the live web!

https://codereview.chromium.org/11772005/diff/20003/content/browser/plugin_se...
content/browser/plugin_service_impl.cc:152: // 'chrome://extensions/' because it
violates the Content Security Policy.
We can remove this TODO.

[nasko, 2013-01-17 22:19:13]

I've removed the changes to the Task Manager, as it was using an incorrect
approach.

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
File chrome/browser/task_manager/task_manager_resource_providers.cc (right):

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1853: // Given a
RenderViewHost, return true if this RVH is Cross-site Iframe.
On 2013/01/17 19:44:54, creis wrote:
> This comment belongs in .h file.
> 
> We should have a separate comment within this function saying why the
heuristic
> it's using (all processes that don't have TabContents) is correct.
> 
> Except... it's not correct.  There are other RenderProcessHosts that don't
have
> TabContents, like background pages and notifications.  We'll need a different
> approach.

We discussed this in person and both agreed this is the wrong approach. This
code is being removed.

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1855: if
(host==NULL || !host->GetHandle())
On 2013/01/17 19:44:54, creis wrote:
> nit: !host

Done.

https://codereview.chromium.org/11772005/diff/20003/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1859: if
((*iterator)->GetRenderProcessHost()->GetHandle() == host->GetHandle())
On 2013/01/17 19:44:54, creis wrote:
> Is it possible for this to crash if a TabContents doesn't have a RPH?

Done.

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (left):

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:156: 
On 2013/01/17 19:44:54, creis wrote:
> No need for this change.

Done.

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:83: // same site
iframes in the same process when running in --site-per-process
On 2013/01/17 19:44:54, creis wrote:
> Can you change this comment a bit?  We're not technically hitting this path
for
> same-site iframes (thanks to your change in ResourceLoader), so this is more
> about putting the cross-site URLs into their respective SiteInstances instead
of
> a special guest SiteInstance for BrowserPlugins.
> 
> Maybe we should start by saying that --site-per-process temporarily breaks
> BrowserPlugins and makes them behave like OOP iframes, with a TODO to fix that
> by splitting up BrowserPlugin?

Done.

https://codereview.chromium.org/11772005/diff/20003/content/browser/child_pro...
File content/browser/child_process_security_policy_impl.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/child_pro...
content/browser/child_process_security_policy_impl.cc:512: // out-of-process
iframes is ready to go.
On 2013/01/17 19:44:54, creis wrote:
> Wasn't this from your other CL?  Did you want to land that first?

As discussed, I've decided to drop the other CL and merge the code here.

https://codereview.chromium.org/11772005/diff/20003/content/browser/loader/re...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/loader/re...
content/browser/loader/resource_loader.cc:480: // Change the MIME type of the
response if --site-per-process flag
On 2013/01/17 19:44:54, creis wrote:
> We should introduce this comment very clearly saying that it's a temporary
> prototype for out-of-process iframes, and explain briefly how it works.  The
> simple comment was fine when it was just for us, but others are going to
wonder
> what it's for if we land it.

Done.

https://codereview.chromium.org/11772005/diff/20003/content/browser/loader/re...
content/browser/loader/resource_loader.cc:481: // is passed and the current
request is for the Sub-Frame page. Avoid
On 2013/01/17 19:44:54, creis wrote:
> nit: the Sub-Frame page -> for a subframe.

Done.

https://codereview.chromium.org/11772005/diff/20003/content/browser/plugin_se...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/plugin_se...
content/browser/plugin_service_impl.cc:150: // Register MIME type to support
WebView component.
On 2013/01/17 19:44:54, creis wrote:
> This needs to be behind the --site-per-process flag.  Don't want this plugin
> available on the live web!

Done.

https://codereview.chromium.org/11772005/diff/20003/content/browser/plugin_se...
content/browser/plugin_service_impl.cc:152: // 'chrome://extensions/' because it
violates the Content Security Policy.
On 2013/01/17 19:44:54, creis wrote:
> We can remove this TODO.

Done.

[Charlie Reis, 2013-01-18 05:37:07]

https://codereview.chromium.org/11772005/diff/20003/content/browser/child_pro...
File content/browser/child_process_security_policy_impl.cc (right):

https://codereview.chromium.org/11772005/diff/20003/content/browser/child_pro...
content/browser/child_process_security_policy_impl.cc:512: // out-of-process
iframes is ready to go.
On 2013/01/17 22:19:13, nasko wrote:
> On 2013/01/17 19:44:54, creis wrote:
> > Wasn't this from your other CL?  Did you want to land that first?
> 
> As discussed, I've decided to drop the other CL and merge the code here.

Ok.  In that case, can you add the bug number from that other CL to this one?

https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manag...
File chrome/browser/task_manager/task_manager_resource_providers.cc (right):

https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1866:
Add(RenderViewHost::From(const_cast<RenderWidgetHost*>(widget)));
Nit: Might as well put this back to avoid the churn.

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:82: // When
--site-per-process is pecified, the behavior of BrowserPlugin
nit: specified

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:84: // iframes. Use
the src URL sent by the renderer to find the right
nit: iframes instead.
nit: We use

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:89: // TODO(nasko):
Fix this such that it doesn't break the browser plugin.
nit: , and such that cross-process navigations are supported.

https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_se...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_se...
content/browser/plugin_service_impl.cc:150: // Register MIME type to support
WebView component.
Great.  Can you add a similar --site-per-process prototype comment here?

https://codereview.chromium.org/11772005/diff/29001/content/common/browser_pl...
File content/common/browser_plugin_messages.h (right):

https://codereview.chromium.org/11772005/diff/29001/content/common/browser_pl...
content/common/browser_plugin_messages.h:59: IPC_STRUCT_MEMBER(std::string, src)
Note: this is also being added by https://codereview.chromium.org/11956022/. 
Not sure which one will land first.

[nasko, 2013-01-18 19:09:41]

Fixed the remaining comments.

Adding fsamuel@ to review browser plugin changes.

https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manag...
File chrome/browser/task_manager/task_manager_resource_providers.cc (right):

https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manag...
chrome/browser/task_manager/task_manager_resource_providers.cc:1866:
Add(RenderViewHost::From(const_cast<RenderWidgetHost*>(widget)));
On 2013/01/18 05:37:07, creis wrote:
> Nit: Might as well put this back to avoid the churn.

Done.

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:82: // When
--site-per-process is pecified, the behavior of BrowserPlugin
On 2013/01/18 05:37:07, creis wrote:
> nit: specified

Done.

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:84: // iframes. Use
the src URL sent by the renderer to find the right
On 2013/01/18 05:37:07, creis wrote:
> nit: iframes instead.
> nit: We use

Done.

https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:89: // TODO(nasko):
Fix this such that it doesn't break the browser plugin.
On 2013/01/18 05:37:07, creis wrote:
> nit: , and such that cross-process navigations are supported.

Done.

https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_se...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_se...
content/browser/plugin_service_impl.cc:150: // Register MIME type to support
WebView component.
On 2013/01/18 05:37:07, creis wrote:
> Great.  Can you add a similar --site-per-process prototype comment here?

Done.

https://codereview.chromium.org/11772005/diff/29001/content/common/browser_pl...
File content/common/browser_plugin_messages.h (right):

https://codereview.chromium.org/11772005/diff/29001/content/common/browser_pl...
content/common/browser_plugin_messages.h:59: IPC_STRUCT_MEMBER(std::string, src)
On 2013/01/18 05:37:07, creis wrote:
> Note: this is also being added by https://codereview.chromium.org/11956022/. 
> Not sure which one will land first.

Done.

[awong, 2013-01-18 22:48:55]

Drive-by:  How are we tracking the TODOs?  Should there be bugs for this?


On 2013/01/18 19:09:41, nasko wrote:
> Fixed the remaining comments.
> 
> Adding fsamuel@ to review browser plugin changes.
> 
>
https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manag...
> File chrome/browser/task_manager/task_manager_resource_providers.cc (right):
> 
>
https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manag...
> chrome/browser/task_manager/task_manager_resource_providers.cc:1866:
> Add(RenderViewHost::From(const_cast<RenderWidgetHost*>(widget)));
> On 2013/01/18 05:37:07, creis wrote:
> > Nit: Might as well put this back to avoid the churn.
> 
> Done.
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
> File content/browser/browser_plugin/browser_plugin_embedder.cc (right):
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
> content/browser/browser_plugin/browser_plugin_embedder.cc:82: // When
> --site-per-process is pecified, the behavior of BrowserPlugin
> On 2013/01/18 05:37:07, creis wrote:
> > nit: specified
> 
> Done.
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
> content/browser/browser_plugin/browser_plugin_embedder.cc:84: // iframes. Use
> the src URL sent by the renderer to find the right
> On 2013/01/18 05:37:07, creis wrote:
> > nit: iframes instead.
> > nit: We use
> 
> Done.
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_p...
> content/browser/browser_plugin/browser_plugin_embedder.cc:89: // TODO(nasko):
> Fix this such that it doesn't break the browser plugin.
> On 2013/01/18 05:37:07, creis wrote:
> > nit: , and such that cross-process navigations are supported.
> 
> Done.
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_se...
> File content/browser/plugin_service_impl.cc (right):
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_se...
> content/browser/plugin_service_impl.cc:150: // Register MIME type to support
> WebView component.
> On 2013/01/18 05:37:07, creis wrote:
> > Great.  Can you add a similar --site-per-process prototype comment here?
> 
> Done.
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/common/browser_pl...
> File content/common/browser_plugin_messages.h (right):
> 
>
https://codereview.chromium.org/11772005/diff/29001/content/common/browser_pl...
> content/common/browser_plugin_messages.h:59: IPC_STRUCT_MEMBER(std::string,
src)
> On 2013/01/18 05:37:07, creis wrote:
> > Note: this is also being added by https://codereview.chromium.org/11956022/.

> > Not sure which one will land first.
> 
> Done.

[nasko, 2013-01-18 22:57:58]

I plan on revisiting all the TODO(nasko) once we have proper oop iframes
support and clean them all up. If you prefer, I can tag them all with the
master oop iframe bug.


On Fri, Jan 18, 2013 at 2:48 PM, <ajwong@chromium.org> wrote:

> Drive-by:  How are we tracking the TODOs?  Should there be bugs for this?
>
>
>
> On 2013/01/18 19:09:41, nasko wrote:
>
>> Fixed the remaining comments.
>>
>
>  Adding fsamuel@ to review browser plugin changes.
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
chrome/browser/task_manager/**task_manager_resource_**providers.cc<https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manager/task_manager_resource_providers.cc>
>
>> File chrome/browser/task_manager/**task_manager_resource_**providers.cc
>> (right):
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
> chrome/browser/task_manager/**task_manager_resource_**
>
providers.cc#newcode1866<https://codereview.chromium.org/11772005/diff/29001/chrome/browser/task_manager/task_manager_resource_providers.cc#newcode1866>
>
>> chrome/browser/task_manager/**task_manager_resource_**providers.cc:1866:
>> Add(RenderViewHost::From(**const_cast<RenderWidgetHost*>(**widget)));
>> On 2013/01/18 05:37:07, creis wrote:
>> > Nit: Might as well put this back to avoid the churn.
>>
>
>  Done.
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/browser/browser_**plugin/browser_plugin_**embedder.cc<https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_plugin/browser_plugin_embedder.cc>
>
>> File content/browser/browser_**plugin/browser_plugin_**embedder.cc
>> (right):
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/browser/browser_**plugin/browser_plugin_**embedder.cc#newcode82<https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_plugin/browser_plugin_embedder.cc#newcode82>
>
>> content/browser/browser_**plugin/browser_plugin_**embedder.cc:82: // When
>> --site-per-process is pecified, the behavior of BrowserPlugin
>> On 2013/01/18 05:37:07, creis wrote:
>> > nit: specified
>>
>
>  Done.
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/browser/browser_**plugin/browser_plugin_**embedder.cc#newcode84<https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_plugin/browser_plugin_embedder.cc#newcode84>
>
>> content/browser/browser_**plugin/browser_plugin_**embedder.cc:84: //
>> iframes. Use
>> the src URL sent by the renderer to find the right
>> On 2013/01/18 05:37:07, creis wrote:
>> > nit: iframes instead.
>> > nit: We use
>>
>
>  Done.
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/browser/browser_**plugin/browser_plugin_**embedder.cc#newcode89<https://codereview.chromium.org/11772005/diff/29001/content/browser/browser_plugin/browser_plugin_embedder.cc#newcode89>
>
>> content/browser/browser_**plugin/browser_plugin_**embedder.cc:89: //
>> TODO(nasko):
>> Fix this such that it doesn't break the browser plugin.
>> On 2013/01/18 05:37:07, creis wrote:
>> > nit: , and such that cross-process navigations are supported.
>>
>
>  Done.
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/browser/plugin_**service_impl.cc<https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_service_impl.cc>
>
>> File content/browser/plugin_**service_impl.cc (right):
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/browser/plugin_**service_impl.cc#newcode150<https://codereview.chromium.org/11772005/diff/29001/content/browser/plugin_service_impl.cc#newcode150>
>
>> content/browser/plugin_**service_impl.cc:150: // Register MIME type to
>> support
>> WebView component.
>> On 2013/01/18 05:37:07, creis wrote:
>> > Great.  Can you add a similar --site-per-process prototype comment here?
>>
>
>  Done.
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/common/browser_plugin_**messages.h<https://codereview.chromium.org/11772005/diff/29001/content/common/browser_plugin_messages.h>
>
>> File content/common/browser_plugin_**messages.h (right):
>>
>
>
> https://codereview.chromium.**org/11772005/diff/29001/**
>
content/common/browser_plugin_**messages.h#newcode59<https://codereview.chromium.org/11772005/diff/29001/content/common/browser_plugin_messages.h#newcode59>
>
>> content/common/browser_plugin_**messages.h:59:
>> IPC_STRUCT_MEMBER(std::string,
>>
> src)
>
>> On 2013/01/18 05:37:07, creis wrote:
>> > Note: this is also being added by https://codereview.chromium.**
>> org/11956022/ <https://codereview.chromium.org/11956022/>.
>>
>
>  > Not sure which one will land first.
>>
>
>  Done.
>>
>
>
>
>
https://codereview.chromium.**org/11772005/<https://codereview.chromium.org/1...
>

[Charlie Reis, 2013-01-23 00:17:37]

Great.  LGTM with nits.

https://codereview.chromium.org/11772005/diff/35002/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/11772005/diff/35002/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_embedder.cc:92:
web_contents()->GetSiteInstance()->GetRelatedSiteInstance(
nit: 2 more spaces needed here.

https://codereview.chromium.org/11772005/diff/35002/content/common/browser_pl...
File content/common/browser_plugin_messages.h (right):

https://codereview.chromium.org/11772005/diff/35002/content/common/browser_pl...
content/common/browser_plugin_messages.h:59: IPC_STRUCT_MEMBER(std::string, src)
https://chromiumcodereview.appspot.com/11956022 has landed, so you'll need to
merge before landing.

[Fady Samuel, 2013-01-23 01:05:41]

browser_plugin LGTM once you merge with ToT.

[commit-bot: I haz the power, 2013-01-23 14:07:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/11772005/50001

[commit-bot: I haz the power, 2013-01-23 15:12:37]

Change committed as 178292