Adding --disable-plugins-discovery command line switch to not load third-party plugins from common …

Adding --disable-plugins-discovery command line switch to not load third-party plugins from common locations.
When --disable-plugins-discovery switch is set, any built-in plugins are still loaded as well as third-party ones that are explicitly requested with --extra-plugin-dir or --load-plugin switches. No other plugins are loaded.

Switch is effective on posix, mac and win platforms.

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

[seva, 2013-06-06 03:46:59]

Hi Bernhard,

Can you please make a code review.

Note the additional change in browser_process_impl.cc to ensure
~/.confog/chromium/Plugins arent loaded as well. It looks add that this
directory is added as an "extra" directory and from there, more naturally it
could be done as part of GetPluginDirectories in plugin_list_posix.cc, but I
suspect there are technical issues with that?

Also I'm not 100% percent confident with my changes being sufficiently
thread-safe..

Cheers,
Seva

[Bernhard Bauer, 2013-06-06 08:28:50]

Getting close, just some smaller changes.

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
Why do you need this as a member variable?

https://codereview.chromium.org/16369004/diff/1/content/browser/plugin_servic...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/16369004/diff/1/content/browser/plugin_servic...
content/browser/plugin_service_impl.cc:200: if
(command_line->HasSwitch(switches::kDisablePluginsDiscovery)) {
Braces aren't required, and Chromium style is to remove them.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list.h (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list.h:68: // Disables discovery of third_party
plugins in common places next time
s/common/well-known/?

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list.h:198: bool plugins_discovery_disabled_;
I don't think this needs to be protected.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list_mac.mm (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_mac.mm:278: if
(PluginList::plugins_discovery_disabled_) {
Remove braces please.

Also, can you make the previous line fit into 80 columns?

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list_posix.cc (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_posix.cc:438: if
(PluginList::plugins_discovery_disabled_) {
Remove braces.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_posix.cc:446: // AddExtraPluginDir call in
chrome/browser/browser_process_impl.cc.
I'm not sure if the comment here should refer to concepts from Chrome. Can you
simply talk about directories added via AddExtraPluginDir()?

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list_win.cc (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_win.cc:309: if
(PluginList::plugins_discovery_disabled_) {
Remove braces please, make the previous line fit into 80 columns.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_win.cc:364: if
(PluginList::plugins_discovery_disabled_) {
Same here.

[seva, 2013-06-06 22:11:24]

Also adding sky@chromium.org, joi@chromium.org as reviewers for content/ and
chrome/ stuff.

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
File chrome/browser/browser_process_impl.cc (right):

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
chrome/browser/browser_process_impl.cc:888: if
(!command_line_.HasSwitch(switches::kDisablePluginsDiscovery)) {
On this note, it feels that this "Plugins" directory should be a part of what
PluginList::GetPluginDirectories() returns.

That way these "if
(!command_line_.HasSwitch(switches::kDisablePluginsDiscovery))" checks are
better localized.

Also, functionally, this directory is being "watched" for file changes and
related functionality works too (See PluginServiceImpl::StartWatchingPlugins(),
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...
).

If what I'm saying makes sense and fixing this isn't too involved I'm happy to
work on addressing; separately from BUG=239325

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> Why do you need this as a member variable?

This saves two CommandLine::ForCurrentProcess() calls and keeps things a bit
cleaner, more testable (BrowserProcessImpl constructor parameter "command_line"
is now used) and easier extensible (command_line is available in case it's
needed for other checks in the future, which seems likely), to my taste.

One CommandLine::ForCurrentProcess() call I removed from browser_process_impl.cc
line 902 and the other I would have to add there for my
HasSwitch(switches::kDisablePluginsDiscovery) check.

Though if you think this has issues (or too unrelated to the intention of the
patch) I'm totally open to reverting this part and just making another
CommandLine::ForCurrentProcess() call in browser_process_impl.cc.

https://codereview.chromium.org/16369004/diff/1/content/browser/plugin_servic...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/16369004/diff/1/content/browser/plugin_servic...
content/browser/plugin_service_impl.cc:200: if
(command_line->HasSwitch(switches::kDisablePluginsDiscovery)) {
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> Braces aren't required, and Chromium style is to remove them.

Done.

https://codereview.chromium.org/16369004/diff/1/content/public/common/content...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/16369004/diff/1/content/public/common/content...
content/public/common/content_switches.h:171: CONTENT_EXPORT extern const char
kDisablePluginsDiscovery[];
BTW, what does "CONTENT_EXPORT" prefix (macros?) do? Some of the constants are
prefixed with that and some not; I failed to find an obvious reason. So I'm
unsure whether this new constant needs it.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list.h (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list.h:68: // Disables discovery of third_party
plugins in common places next time
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> s/common/well-known/?

I don't know how well known these are :) everything is relative :)
How about "standard"?

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list.h:198: bool plugins_discovery_disabled_;
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> I don't think this needs to be protected.

Indeed; moved down to private: section. Thanks.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list_mac.mm (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_mac.mm:278: if
(PluginList::plugins_discovery_disabled_) {
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> Remove braces please.
> 
> Also, can you make the previous line fit into 80 columns?

Done.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list_posix.cc (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_posix.cc:438: if
(PluginList::plugins_discovery_disabled_) {
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> Remove braces.

Done.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_posix.cc:446: // AddExtraPluginDir call in
chrome/browser/browser_process_impl.cc.
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> I'm not sure if the comment here should refer to concepts from Chrome. Can you
> simply talk about directories added via AddExtraPluginDir()?

Done.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list_win.cc (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_win.cc:309: if
(PluginList::plugins_discovery_disabled_) {
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> Remove braces please, make the previous line fit into 80 columns.

Done.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list_win.cc:364: if
(PluginList::plugins_discovery_disabled_) {
On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> Same here.

Done.

[Jói, 2013-06-06 22:14:24]

content_switches changes LGTM

[seva, 2013-06-07 03:11:43]

Speaking of testing. I tested this under Linux; manually. Is it good enough if I
test under Mac and Windows after commit? E.g. I would get and manually test
first Mac and Windows "official" nightly builds containing my change.

[Bernhard Bauer, 2013-06-07 10:33:43]

LGTM! Just one nit at the end.

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
File chrome/browser/browser_process_impl.cc (right):

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
chrome/browser/browser_process_impl.cc:888: if
(!command_line_.HasSwitch(switches::kDisablePluginsDiscovery)) {
On 2013/06/06 22:11:25, sevalo wrote:
> On this note, it feels that this "Plugins" directory should be a part of what
> PluginList::GetPluginDirectories() returns.
> 
> That way these "if
> (!command_line_.HasSwitch(switches::kDisablePluginsDiscovery))" checks are
> better localized.

Well, this directory is not being returned by default by PluginList because it's
meant to contain plug-ins that are specific to Chrome. 

PluginList is part of the general framework for a multi-process browser
("content"), which shouldn't contain Chrome-specific code. The other plug-in
locations are used by other browsers (Firefox and Safari on Mac, specifically).

That being said, ...

> Also, functionally, this directory is being "watched" for file changes and
> related functionality works too (See
PluginServiceImpl::StartWatchingPlugins(),
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...
> ).

...hmm, it would indeed make sense for PluginList::GetPluginDirectories() to
return extra plug-in directories as well. That would make watching for changes
work automatically, and we'd need to check the switch only once.

> If what I'm saying makes sense and fixing this isn't too involved I'm happy to
> work on addressing; separately from BUG=239325

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/16369004/diff/1/chrome/browser/browser_proces...
chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
On 2013/06/06 22:11:25, sevalo wrote:
> On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> > Why do you need this as a member variable?
> 
> This saves two CommandLine::ForCurrentProcess() calls and keeps things a bit
> cleaner, more testable (BrowserProcessImpl constructor parameter
"command_line"
> is now used) and easier extensible (command_line is available in case it's
> needed for other checks in the future, which seems likely), to my taste.
> 
> One CommandLine::ForCurrentProcess() call I removed from
browser_process_impl.cc
> line 902 and the other I would have to add there for my
> HasSwitch(switches::kDisablePluginsDiscovery) check.
> 
> Though if you think this has issues (or too unrelated to the intention of the
> patch) I'm totally open to reverting this part and just making another
> CommandLine::ForCurrentProcess() call in browser_process_impl.cc.

No, I think it's fine. I was going to say that this isn't tested very
extensively anyway (and if it was, it's not much more difficult to override the
current command line temporarily), but I didn't see that it already gets a
command line as a construction parameter. In that case it definitely makes sense
to keep it around, to make sure we always use the same instance.

https://codereview.chromium.org/16369004/diff/1/content/public/common/content...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/16369004/diff/1/content/public/common/content...
content/public/common/content_switches.h:171: CONTENT_EXPORT extern const char
kDisablePluginsDiscovery[];
On 2013/06/06 22:11:25, sevalo wrote:
> BTW, what does "CONTENT_EXPORT" prefix (macros?) do? Some of the constants are
> prefixed with that and some not; I failed to find an obvious reason. So I'm
> unsure whether this new constant needs it.

One of the configurations we build Chrome in is a shared library build. On
Windows, that means multiple DLLs (chrome, content, etc.). Now, DLLs need to
declare the symbols they're exporting, otherwise you can't link against them. At
the same time, if you want to use a symbol from a different library, you need to
import it. The FOO_EXPORT macros let you do that, depending on whether you
include a file from inside or outside of the FOO DLL. What this means
effectively is that if you want to reference the flag outside of content/ (which
we do want in this case), we need to add the macro.

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
File webkit/plugins/npapi/plugin_list.h (right):

https://codereview.chromium.org/16369004/diff/1/webkit/plugins/npapi/plugin_l...
webkit/plugins/npapi/plugin_list.h:68: // Disables discovery of third_party
plugins in common places next time
On 2013/06/06 22:11:25, sevalo wrote:
> On 2013/06/06 08:28:50, Bernhard Bauer wrote:
> > s/common/well-known/?
> 
> I don't know how well known these are :) everything is relative :)
> How about "standard"?

Standard is relative as well, I guess ;-)

But "standard" is fine with me, actually. I just don't like "common" here
because it has too many meanings (shared? ordinary?).

https://codereview.chromium.org/16369004/diff/14002/webkit/plugins/npapi/plug...
File webkit/plugins/npapi/plugin_list_win.cc (right):

https://codereview.chromium.org/16369004/diff/14002/webkit/plugins/npapi/plug...
webkit/plugins/npapi/plugin_list_win.cc:363: void
PluginList::GetPluginPathsFromRegistry(std::vector<base::FilePath>* plugins) {
Can you make this fit into 80 columns please?

(I guess there was a refactoring here to move FilePath into the base namespace
that pushed these over the limit.)

[sky, 2013-06-07 17:22:51]

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
I get worried about lifetime when objects keep references like this. Is there a
reason you can't look up the CommandLine as necessary when you need it?

[seva, 2013-06-07 19:02:35]

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
On 2013/06/07 17:22:51, sky wrote:
> I get worried about lifetime when objects keep references like this. Is there
a
> reason you can't look up the CommandLine as necessary when you need it?

I really have no educated opinion here, I'm not very familiar with this code. I
simply noticed there's a constructor parameter (I assumed there's a good reason
for that), and that command line actually is passed there, and that constructor
ignores that passed value. And that in one place command gets looked up, and I
would need to add another lookup. And I thought that's not perfectly clean, so I
did this change. Manual testing showed that it works.

I trust you making a decision whether this change is to the better. Let me know
and I will revert (and look up command line where I need it).

https://codereview.chromium.org/16369004/diff/14002/webkit/plugins/npapi/plug...
File webkit/plugins/npapi/plugin_list_win.cc (right):

https://codereview.chromium.org/16369004/diff/14002/webkit/plugins/npapi/plug...
webkit/plugins/npapi/plugin_list_win.cc:363: void
PluginList::GetPluginPathsFromRegistry(std::vector<base::FilePath>* plugins) {
On 2013/06/07 10:33:43, Bernhard Bauer wrote:
> Can you make this fit into 80 columns please?
> 
> (I guess there was a refactoring here to move FilePath into the base namespace
> that pushed these over the limit.)

Done.

[sky, 2013-06-07 19:48:55]

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
On 2013/06/07 19:02:36, sevalo wrote:
> On 2013/06/07 17:22:51, sky wrote:
> > I get worried about lifetime when objects keep references like this. Is
there
> a
> > reason you can't look up the CommandLine as necessary when you need it?
> 
> I really have no educated opinion here, I'm not very familiar with this code.
I
> simply noticed there's a constructor parameter (I assumed there's a good
reason
> for that), and that command line actually is passed there, and that
constructor
> ignores that passed value. And that in one place command gets looked up, and I
> would need to add another lookup. And I thought that's not perfectly clean, so
I
> did this change. Manual testing showed that it works.
> 
> I trust you making a decision whether this change is to the better. Let me
know
> and I will revert (and look up command line where I need it).

I'm not sure why we pass in the CommandLine here either. Maybe Joi knows?

[Jói, 2013-06-08 14:35:31]

I don't know off the top of my head, but I wouldn't keep the
reference, just look the command line up as was done before.

On Fri, Jun 7, 2013 at 8:48 PM,  <sky@chromium.org> wrote:
>
>
https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
> File chrome/browser/browser_process_impl.h (right):
>
>
https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
> chrome/browser/browser_process_impl.h:252: const CommandLine&
> command_line_;
> On 2013/06/07 19:02:36, sevalo wrote:
>>
>> On 2013/06/07 17:22:51, sky wrote:
>> > I get worried about lifetime when objects keep references like this.
>
> Is there
>>
>> a
>> > reason you can't look up the CommandLine as necessary when you need
>
> it?
>
>> I really have no educated opinion here, I'm not very familiar with
>
> this code. I
>>
>> simply noticed there's a constructor parameter (I assumed there's a
>
> good reason
>>
>> for that), and that command line actually is passed there, and that
>
> constructor
>>
>> ignores that passed value. And that in one place command gets looked
>
> up, and I
>>
>> would need to add another lookup. And I thought that's not perfectly
>
> clean, so I
>>
>> did this change. Manual testing showed that it works.
>
>
>> I trust you making a decision whether this change is to the better.
>
> Let me know
>>
>> and I will revert (and look up command line where I need it).
>
>
> I'm not sure why we pass in the CommandLine here either. Maybe Joi
> knows?
>
> https://codereview.chromium.org/16369004/

[seva, 2013-06-11 03:02:31]

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
File chrome/browser/browser_process_impl.h (right):

https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
On 2013/06/07 19:48:56, sky wrote:
> On 2013/06/07 19:02:36, sevalo wrote:
> > On 2013/06/07 17:22:51, sky wrote:
> > > I get worried about lifetime when objects keep references like this. Is
> there
> > a
> > > reason you can't look up the CommandLine as necessary when you need it?
> > 
> > I really have no educated opinion here, I'm not very familiar with this
code.
> I
> > simply noticed there's a constructor parameter (I assumed there's a good
> reason
> > for that), and that command line actually is passed there, and that
> constructor
> > ignores that passed value. And that in one place command gets looked up, and
I
> > would need to add another lookup. And I thought that's not perfectly clean,
so
> I
> > did this change. Manual testing showed that it works.
> > 
> > I trust you making a decision whether this change is to the better. Let me
> know
> > and I will revert (and look up command line where I need it).
> 
> I'm not sure why we pass in the CommandLine here either. Maybe Joi knows?

Done.
Now browser_process_impl.h is completely reverted and browser_process_impl.cc
changes minimally.

Patch sets 5 and 7 are applying this change (sorry for the extra patch).

Patch set 6 is a sync to
https://commondatastorage.googleapis.com/chromium-browser-continuous/Linux/LA...

Once it looks good to all reviewers, Bernhard, may I ask you to commit this?

[commit-bot: I haz the power, 2013-06-11 05:59:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sevalo@google.com/16369004/60001

[Bernhard Bauer, 2013-06-11 06:00:33]

On 2013/06/11 03:02:31, seva wrote:
>
https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
> File chrome/browser/browser_process_impl.h (right):
> 
>
https://codereview.chromium.org/16369004/diff/14002/chrome/browser/browser_pr...
> chrome/browser/browser_process_impl.h:252: const CommandLine& command_line_;
> On 2013/06/07 19:48:56, sky wrote:
> > On 2013/06/07 19:02:36, sevalo wrote:
> > > On 2013/06/07 17:22:51, sky wrote:
> > > > I get worried about lifetime when objects keep references like this. Is
> > there
> > > a
> > > > reason you can't look up the CommandLine as necessary when you need it?
> > > 
> > > I really have no educated opinion here, I'm not very familiar with this
> code.
> > I
> > > simply noticed there's a constructor parameter (I assumed there's a good
> > reason
> > > for that), and that command line actually is passed there, and that
> > constructor
> > > ignores that passed value. And that in one place command gets looked up,
and
> I
> > > would need to add another lookup. And I thought that's not perfectly
clean,
> so
> > I
> > > did this change. Manual testing showed that it works.
> > > 
> > > I trust you making a decision whether this change is to the better. Let me
> > know
> > > and I will revert (and look up command line where I need it).
> > 
> > I'm not sure why we pass in the CommandLine here either. Maybe Joi knows?
> 
> Done.
> Now browser_process_impl.h is completely reverted and browser_process_impl.cc
> changes minimally.
> 
> Patch sets 5 and 7 are applying this change (sorry for the extra patch).
> 
> Patch set 6 is a sync to
>
https://commondatastorage.googleapis.com/chromium-browser-continuous/Linux/LA...
> 
> Once it looks good to all reviewers, Bernhard, may I ask you to commit this?

You can do that yourself by checking the "commit" checkbox.

[commit-bot: I haz the power, 2013-06-11 08:34:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sevalo@google.com/16369004/60001

[commit-bot: I haz the power, 2013-06-11 08:44:22]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[seva, 2013-06-11 22:48:13]

Sky, I have addressed your last comment, can you please have another look? I
need your approval power in chrome/browser and content/browser packages to be
able to submit this change. Thank you for review!

[sky, 2013-06-11 23:26:54]

LGTM

[commit-bot: I haz the power, 2013-06-11 23:51:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sevalo@google.com/16369004/60001

[commit-bot: I haz the power, 2013-06-12 04:24:37]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-06-12 17:24:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sevalo@google.com/16369004/60001

[commit-bot: I haz the power, 2013-06-12 17:30:49]

Change committed as 205850