Move pref names and default value into WebsiteSettingsInfo

Move pref names and default value into WebsiteSettingsInfo

This moves content setting pref names and their default values into WebsiteSettingsInfo.

BUG=512683
Committed: https://crrev.com/bc7409e2b4d2fea26e8d1c7b35132a301aba4504
Cr-Commit-Position: refs/heads/master@{#342045}

[raymes, 2015-07-29 07:36:35]

raymes@chromium.org changed reviewers:
+ bauerb@chromium.org, msramek@chromium.org

[raymes, 2015-07-29 07:36:36]

PTAL :)

[msramek, 2015-07-29 09:20:07]

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_info.cc (right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.cc:28:
base::Value* initial_default_value)
It seems a bit unnecessary that we convert ContentSetting to base::Value in
RegisterPermission(), then back to int in this constructor. Then as a consumer,
I would convert that int back to ContentSetting which is more meaningful.

I understand that this is because you want to support arbitrary default values.
Wouldn't it then be better to store the default value as base::Value, and make
getters

base::Value* default_value()
ContentSetting default_permission()
base::DictionaryValue* default_dictionary()

where for each content type one of the latter two would fail (or return
CONTENT_SETTING_DEFAULT/nullptr) depending on if it is permissiony?

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_info.h (right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.h:44: int32_t
initial_default_value_;
Just int? This is mapped to Prefs::GetInteger(), resp.
base::Value::GetAsInteger(), both of them work with ints.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_registry.h
(right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_registry.h:39: //
Register a new WebsiteSettingsInfo for a permission-like setting.
In our past discussions, we used the word "permission" to refer to media,
location etc. as opposed to e.g. images (which is very much a *content setting*,
not a requested permission), so this is not so clear.

Please expand the comment to explain what is permission-like, i.e. has a scalar
ALLOW / ASK / BLOCK / ... value and not an arbitrary dictionary.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/common/pref_names.h (left):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/common/pref_names.h:16: extern const char
kDefaultCookiesSetting[];
Nit: Please leave a comment that this file now doesn't contain all content
settings-related prefs, and that defaults and exceptions are now located in
WebsiteSettingsRegistry.

[Bernhard Bauer, 2015-07-29 10:12:39]

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
File chrome/browser/prefs/pref_model_associator.cc (right):

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_model_associator.cc:332: return
scoped_ptr<base::Value>(
Can you use make_scoped_ptr()?

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_model_associator.cc:334: .Pass();
The .Pass() is only necessary to pass ownership from a scoped_ptr on the stack.
If it's the return value of a function, it already is an rvalue.

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_model_associator.cc:339: return
scoped_ptr<base::Value>(server_value.DeepCopy()).Pass();
Same here.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File
components/content_settings/core/browser/content_settings_default_provider.cc
(right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/content_settings_default_provider.cc:48:
std::string GetPrefName(ContentSettingsType type) {
You could return this as const-ref.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_info.h (right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.h:23: //
Ownership of |default_value| remains with the caller.
Should be |initial_default_value| (although, isn't that redundant? Can the
default value change?). Also, can you pass the pointer as const?

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.h:44: int32_t
initial_default_value_;
On 2015/07/29 09:20:07, msramek wrote:
> Just int? This is mapped to Prefs::GetInteger(), resp.
> base::Value::GetAsInteger(), both of them work with ints.

Yes please. The style guide says to use int unless you need the value to be
exactly 32 bits in size.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_registry.h
(right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_registry.h:39: //
Register a new WebsiteSettingsInfo for a permission-like setting.
On 2015/07/29 09:20:07, msramek wrote:
> In our past discussions, we used the word "permission" to refer to media,
> location etc. as opposed to e.g. images (which is very much a *content
setting*,
> not a requested permission), so this is not so clear.
> 
> Please expand the comment to explain what is permission-like, i.e. has a
scalar
> ALLOW / ASK / BLOCK / ... value and not an arbitrary dictionary.

In the past, we've used the term "compound" for content types that can't be
mapped to a ContentSetting (because they're a ListValue or DictionaryValue).

[raymes, 2015-07-30 05:25:03]

Thanks!

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
File chrome/browser/prefs/pref_model_associator.cc (right):

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_model_associator.cc:332: return
scoped_ptr<base::Value>(
On 2015/07/29 10:12:38, Bernhard Bauer wrote:
> Can you use make_scoped_ptr()?

Done.

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_model_associator.cc:334: .Pass();
On 2015/07/29 10:12:38, Bernhard Bauer wrote:
> The .Pass() is only necessary to pass ownership from a scoped_ptr on the
stack.
> If it's the return value of a function, it already is an rvalue.

Thanks - I wasn't really thinking about this as I was just blindly converting
the code but good catch :)

https://codereview.chromium.org/1252073002/diff/80001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_model_associator.cc:339: return
scoped_ptr<base::Value>(server_value.DeepCopy()).Pass();
On 2015/07/29 10:12:38, Bernhard Bauer wrote:
> Same here.

Done.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File
components/content_settings/core/browser/content_settings_default_provider.cc
(right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/content_settings_default_provider.cc:48:
std::string GetPrefName(ContentSettingsType type) {
On 2015/07/29 10:12:38, Bernhard Bauer wrote:
> You could return this as const-ref.

Done.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_info.cc (right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.cc:28:
base::Value* initial_default_value)
On 2015/07/29 09:20:07, msramek wrote:
> It seems a bit unnecessary that we convert ContentSetting to base::Value in
> RegisterPermission(), then back to int in this constructor. Then as a
consumer,
> I would convert that int back to ContentSetting which is more meaningful.
> 
> I understand that this is because you want to support arbitrary default
values.
> Wouldn't it then be better to store the default value as base::Value, and make
> getters
> 
> base::Value* default_value()
> ContentSetting default_permission()
> base::DictionaryValue* default_dictionary()
> 
> where for each content type one of the latter two would fail (or return
> CONTENT_SETTING_DEFAULT/nullptr) depending on if it is permissiony?

Done. Since it's only used in one place I haven't added the getter. Also I'd
prefer to try to keep content settings specific stuff (i.e. stuff that's related
to permission-like things) separate from this class if possible and instead
build an abstraction on top.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_info.h (right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.h:23: //
Ownership of |default_value| remains with the caller.
On 2015/07/29 10:12:38, Bernhard Bauer wrote:
> Should be |initial_default_value| (although, isn't that redundant? Can the
> default value change?). Also, can you pass the pointer as const?

Yeah, the default value can change. This is the initial value that the pref will
be set to. But that can then be changed later by the user. That's why I wanted
to make it explicit :)

I changed the pointer.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.h:44: int32_t
initial_default_value_;
Thanks. Sorry about this - this is something I'm going to have to get used to.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_registry.h
(right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_registry.h:39: //
Register a new WebsiteSettingsInfo for a permission-like setting.
I've named them RegisterWebsiteSetting and RegisterContentSetting because I
think this is also how we refer to them currently. Website settings have
arbitrary data and content settings have allow/ask/block/etc. WDYT?

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/common/pref_names.h (left):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/common/pref_names.h:16: extern const char
kDefaultCookiesSetting[];
On 2015/07/29 09:20:07, msramek wrote:
> Nit: Please leave a comment that this file now doesn't contain all content
> settings-related prefs, and that defaults and exceptions are now located in
> WebsiteSettingsRegistry.

Done.

[Bernhard Bauer, 2015-07-30 08:13:00]

https://codereview.chromium.org/1252073002/diff/140001/chrome/browser/content...
File chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
(right):

https://codereview.chromium.org/1252073002/diff/140001/chrome/browser/content...
chrome/browser/content_settings/content_settings_pref_provider_unittest.cc:49:
pref_->lock_.Release();
Why this change?

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
File
components/content_settings/core/browser/content_settings_default_provider.cc
(right):

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
components/content_settings/core/browser/content_settings_default_provider.cc:51:
NOTREACHED();
If this code path can be reached (in the absence of a bug), don't put a
NOTREACHED() here, but handle it even in a Debug build. Or if this is really not
supposed to happen, just DCHECK the result. See
http://dev.chromium.org/developers/coding-style/#TOC-CHECK-DCHECK-and-NOTREAC...

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
File components/content_settings/core/browser/website_settings_info.cc (right):

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
components/content_settings/core/browser/website_settings_info.cc:39:
DCHECK(initial_default_value_->IsType(base::Value::TYPE_INTEGER));
This is going to crash if a nullptr is passed in, no?

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
File components/content_settings/core/browser/website_settings_info.h (right):

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
components/content_settings/core/browser/website_settings_info.h:27: const
base::Value* initial_default_value);
You could also pass this in as a scoped_ptr, if you are going to make a copy of
it anyway. If I see this correctly, you have two call sites, one of which passes
in nullptr, and the other one a pointer to a just allocated object, which then
gets copied. That way you could save that copy, and the special casing for
nullptr.

[msramek, 2015-07-31 15:27:50]

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_info.cc (right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_info.cc:28:
base::Value* initial_default_value)
On 2015/07/30 05:25:03, raymes wrote:
> On 2015/07/29 09:20:07, msramek wrote:
> > It seems a bit unnecessary that we convert ContentSetting to base::Value in
> > RegisterPermission(), then back to int in this constructor. Then as a
> consumer,
> > I would convert that int back to ContentSetting which is more meaningful.
> > 
> > I understand that this is because you want to support arbitrary default
> values.
> > Wouldn't it then be better to store the default value as base::Value, and
make
> > getters
> > 
> > base::Value* default_value()
> > ContentSetting default_permission()
> > base::DictionaryValue* default_dictionary()
> > 
> > where for each content type one of the latter two would fail (or return
> > CONTENT_SETTING_DEFAULT/nullptr) depending on if it is permissiony?
> 
> Done. Since it's only used in one place I haven't added the getter. Also I'd
> prefer to try to keep content settings specific stuff (i.e. stuff that's
related
> to permission-like things) separate from this class if possible and instead
> build an abstraction on top.

Abstraction on top for permission-like things also sounds good.

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
File components/content_settings/core/browser/website_settings_registry.h
(right):

https://codereview.chromium.org/1252073002/diff/80001/components/content_sett...
components/content_settings/core/browser/website_settings_registry.h:39: //
Register a new WebsiteSettingsInfo for a permission-like setting.
On 2015/07/30 05:25:03, raymes wrote:
> I've named them RegisterWebsiteSetting and RegisterContentSetting because I
> think this is also how we refer to them currently. Website settings have
> arbitrary data and content settings have allow/ask/block/etc. WDYT?

That's the terminology from this document, right?
https://docs.google.com/document/d/1U8S5k9WRFC53R611CRGAlYVuHNxjZGph3WPonkCTuzk/

So SGTM. ( personally, I've been calling them content settings the whole time,
since that's how everything in the code is named. :) )

https://codereview.chromium.org/1252073002/diff/140001/chrome/browser/content...
File chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
(right):

https://codereview.chromium.org/1252073002/diff/140001/chrome/browser/content...
chrome/browser/content_settings/content_settings_pref_provider_unittest.cc:49:
pref_->lock_.Release();
On 2015/07/30 08:13:00, Bernhard Bauer wrote:
> Why this change?

This is apparently rebased over my https://codereview.chromium.org/1257093002/
(but the branch is set to track master, so all changes are visible).

(that said, sorry to delay you, but I'm still waiting for the review of that CL)

[raymes, 2015-08-03 03:18:05]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[raymes, 2015-08-03 03:18:17]

https://codereview.chromium.org/1252073002/diff/140001/chrome/browser/content...
File chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
(right):

https://codereview.chromium.org/1252073002/diff/140001/chrome/browser/content...
chrome/browser/content_settings/content_settings_pref_provider_unittest.cc:49:
pref_->lock_.Release();
Sorry! I've updated the patch to base it on Martin's.

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
File
components/content_settings/core/browser/content_settings_default_provider.cc
(right):

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
components/content_settings/core/browser/content_settings_default_provider.cc:51:
NOTREACHED();
On 2015/07/30 08:13:00, Bernhard Bauer wrote:
> If this code path can be reached (in the absence of a bug), don't put a
> NOTREACHED() here, but handle it even in a Debug build. Or if this is really
not
> supposed to happen, just DCHECK the result. See
>
http://dev.chromium.org/developers/coding-style/#TOC-CHECK-DCHECK-and-NOTREAC...

Done.

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
File components/content_settings/core/browser/website_settings_info.cc (right):

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
components/content_settings/core/browser/website_settings_info.cc:39:
DCHECK(initial_default_value_->IsType(base::Value::TYPE_INTEGER));
On 2015/07/30 08:13:00, Bernhard Bauer wrote:
> This is going to crash if a nullptr is passed in, no?

oops, fixed.

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
File components/content_settings/core/browser/website_settings_info.h (right):

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
components/content_settings/core/browser/website_settings_info.h:27: const
base::Value* initial_default_value);
Let me know if I've done what you were thinking :)

[commit-bot: I haz the power, 2015-08-03 03:18:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/180001

[commit-bot: I haz the power, 2015-08-03 03:18:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-03 03:18:32]

Dry run: CLs for remote refs other than refs/pending/heads/master must contain
NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them

[Bernhard Bauer, 2015-08-03 07:33:52]

LGTM with a last nit:

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
File components/content_settings/core/browser/website_settings_info.h (right):

https://codereview.chromium.org/1252073002/diff/140001/components/content_set...
components/content_settings/core/browser/website_settings_info.h:27: const
base::Value* initial_default_value);
On 2015/08/03 03:18:17, raymes wrote:
> Let me know if I've done what you were thinking :)

Yup, exactly.

https://codereview.chromium.org/1252073002/diff/180001/components/content_set...
File components/content_settings/core/browser/website_settings_info.cc (right):

https://codereview.chromium.org/1252073002/diff/180001/components/content_set...
components/content_settings/core/browser/website_settings_info.cc:34:
initial_default_value_(initial_default_value.release()) {
Use .Pass()

[raymes, 2015-08-03 07:41:42]

Thanks!

https://codereview.chromium.org/1252073002/diff/180001/components/content_set...
File components/content_settings/core/browser/website_settings_info.cc (right):

https://codereview.chromium.org/1252073002/diff/180001/components/content_set...
components/content_settings/core/browser/website_settings_info.cc:34:
initial_default_value_(initial_default_value.release()) {
On 2015/08/03 07:33:52, Bernhard Bauer wrote:
> Use .Pass()

Done.

[msramek, 2015-08-04 15:01:15]

https://codereview.chromium.org/1257093002/ has finally landed, so you should be
unblocked on this. Sorry for the delays (and LGTM)!

[raymes, 2015-08-04 22:55:08]

The CQ bit was checked by raymes@chromium.org

[raymes, 2015-08-04 22:55:08]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1252073002/#ps200001
(title: " ")

[commit-bot: I haz the power, 2015-08-04 22:55:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/200001

[commit-bot: I haz the power, 2015-08-04 22:55:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-04 22:55:34]

CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true
and NOPRESUBMIT=true in order for the CQ to process them

[raymes, 2015-08-05 03:02:25]

raymes@chromium.org changed reviewers:
+ benwells@chromium.org, mlamouri@chromium.org

[raymes, 2015-08-05 03:02:26]

+OWNERS

+mlamouri for chrome/browser/permissions/permission_context_base.cc
+benwells for chrome/common/extensions/api/content_settings.json

[raymes, 2015-08-05 03:02:30]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-05 03:02:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/220001

[commit-bot: I haz the power, 2015-08-05 03:26:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-05 03:26:11]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[raymes, 2015-08-05 05:58:53]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-05 05:59:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/220002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/220002

[commit-bot: I haz the power, 2015-08-05 06:11:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-05 06:11:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[benwells, 2015-08-05 06:11:56]

lgtm

c/b/e lgtm

[raymes, 2015-08-05 06:35:27]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-05 06:35:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/220002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/220002

[commit-bot: I haz the power, 2015-08-05 06:43:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-05 06:43:04]

Dry run: Try jobs failed on following builders:
  linux_arm_compile on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_arm_compi...)
  mac_chromium_10.10_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_10.1...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[mlamouri (slow - plz ping), 2015-08-05 08:47:06]

lgtm

[msramek, 2015-08-05 10:01:29]

The CQ bit was checked by msramek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-05 10:02:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/230021
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/230021

[commit-bot: I haz the power, 2015-08-05 11:15:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-05 11:15:07]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[raymes, 2015-08-06 00:16:55]

The CQ bit was checked by raymes@chromium.org

[raymes, 2015-08-06 00:16:55]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
msramek@chromium.org, benwells@chromium.org, mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1252073002/#ps280001
(title: " ")

[commit-bot: I haz the power, 2015-08-06 00:18:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/280001

[commit-bot: I haz the power, 2015-08-06 00:30:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-06 00:30:30]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[raymes, 2015-08-06 01:09:27]

The CQ bit was checked by raymes@chromium.org

[raymes, 2015-08-06 01:09:28]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
msramek@chromium.org, benwells@chromium.org, mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1252073002/#ps300001
(title: " ")

[commit-bot: I haz the power, 2015-08-06 01:10:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1252073002/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1252073002/300001

[commit-bot: I haz the power, 2015-08-06 01:57:54]

Committed patchset #17 (id:300001)

[commit-bot: I haz the power, 2015-08-06 01:58:39]

Patchset 17 (id:??) landed as
https://crrev.com/bc7409e2b4d2fea26e8d1c7b35132a301aba4504
Cr-Commit-Position: refs/heads/master@{#342045}