Added experiments section and exported the list of SafeBrowsing features in WebUI

-Added the experimental features section as part of the chrome://safe-browsing. For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed: https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a97bc075fd89f5

BUG=740955, 734667
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485821}
Committed: https://chromium.googlesource.com/chromium/src/+/422a878152e9cbada622e6e7f6cfc98d60af96b0

[hkamila, 2017-06-30 22:59:00]

Fixed the rebasing issues

[hkamila, 2017-06-30 23:17:06]

Removed the not needed files and headers

[hkamila, 2017-06-30 23:17:31]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-30 23:17:51]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hkamila, 2017-06-30 23:29:30]

Description was changed from

==========
Export the list of SafeBrowsing features in webui

BUG=
==========

to

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.

BUG=2940963002
==========

[hkamila, 2017-06-30 23:29:30]

hkamila@google.com changed reviewers:
+ jialiul@chromium.org, vakh@google.com

[hkamila, 2017-06-30 23:29:31]

hkamila@google.com changed required reviewers:
+ jialiul@chromium.org, vakh@google.com

[commit-bot: I haz the power, 2017-06-30 23:30:17]

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

[commit-bot: I haz the power, 2017-06-30 23:30:18]

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

[hkamila, 2017-06-30 23:30:33]

Description was changed from

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.

BUG=2940963002
==========

to

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
==========

[hkamila, 2017-07-05 19:42:25]

Fixed formatting

[hkamila, 2017-07-05 19:43:07]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-05 19:43:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-05 20:05:16]

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

[commit-bot: I haz the power, 2017-07-05 20:05:17]

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

[hkamila, 2017-07-05 20:38:37]

Fixed tests

[hkamila, 2017-07-05 20:38:50]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-05 20:39:10]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-05 20:57:43]

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

[commit-bot: I haz the power, 2017-07-05 20:57:43]

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

[hkamila, 2017-07-05 20:59:23]

Fixed tests 2

[hkamila, 2017-07-05 20:59:32]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-05 20:59:55]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hkamila, 2017-07-05 21:22:34]

hkamila@google.com changed reviewers:
+ vakh@chromium.org
- vakh@google.com

[hkamila, 2017-07-05 21:22:34]

hkamila@google.com changed required reviewers:
+ vakh@chromium.org
- vakh@google.com

[hkamila, 2017-07-05 21:23:35]

hkamila@google.com changed reviewers:
+ blundell@chromium.org

[hkamila, 2017-07-05 21:23:35]

blundell@chromium.org: Please review changes in 

components/resources/safe_browsing_resources.grdp

[commit-bot: I haz the power, 2017-07-05 23:00:59]

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

[commit-bot: I haz the power, 2017-07-05 23:01:00]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_TIMED_OUT, build has not
started yet; builder either lacks capacity or does not exist (misspelled?))
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[hkamila, 2017-07-06 00:32:57]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 00:33:07]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 01:53:54]

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

[commit-bot: I haz the power, 2017-07-06 01:53:55]

Dry run: This issue passed the CQ dry run.

[blundell, 2017-07-06 06:54:18]

lgtm

[Jialiu Lin, 2017-07-06 16:52:39]

High level comments:
It feels a little bit counter-intuitive to put feature definition in the
components/safe_browsing/web_ui/*.  
And we probably don't need to introduce a new namespace called "feature" either.


vakh@, hkamila@ what do you think?

[hkamila, 2017-07-06 17:13:37]

On 2017/07/06 16:52:39, Jialiu Lin wrote:
> High level comments:
> It feels a little bit counter-intuitive to put feature definition in the
> components/safe_browsing/web_ui/*.  
> And we probably don't need to introduce a new namespace called "feature"
either.
> 
> 
> vakh@, hkamila@ what do you think?

Agree, will move the files to safe_browsing.

[hkamila, 2017-07-06 23:23:38]

Changed the location of features.cc/.h

[hkamila, 2017-07-06 23:24:28]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 23:24:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hkamila, 2017-07-06 23:27:45]

On 2017/07/06 17:13:37, hkamila wrote:
> On 2017/07/06 16:52:39, Jialiu Lin wrote:
> > High level comments:
> > It feels a little bit counter-intuitive to put feature definition in the
> > components/safe_browsing/web_ui/*.  
> > And we probably don't need to introduce a new namespace called "feature"
> either.
> > 
> > 
> > vakh@, hkamila@ what do you think?
> 
> Agree, will move the files to safe_browsing.
-----
Thank you!
I moved the files to "safe_browsing/". Looking forward to the review.

[commit-bot: I haz the power, 2017-07-06 23:38:18]

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

[commit-bot: I haz the power, 2017-07-06 23:38:20]

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

[Jialiu Lin, 2017-07-06 23:58:28]

Looking good!

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/features.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:4: #include
"components/safe_browsing/features.h"
nit: empty line

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:9: #include "base/feature_list.h"
nit:empty line

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:12: 
safe_browsing namespace?

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/features.h (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:5: #define
COMPONENTS_SAFE_BROWSING_FEATURES_H_
nit: Add an empty line under #define

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:10: #include "base/feature_list.h"
nit: a blank line before #include "base/feature_list.h"

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:16: 
maybe put this class into safe_browsing namespace?

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/web_ui/safe_browsing_ui.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.cc:20: 
safe_browsing namespace

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/web_ui/safe_browsing_ui.h (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.h:18: 
maybe put theses 2 classes into safe_browsing namespace?

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing_db/database_manager.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/database_manager.cc:6: #include
"base/metrics/histogram_macros.h"
nit: please restore the deleted line.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing_db/v4_feature_list.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/v4_feature_list.cc:5: #include
"components/safe_browsing_db/v4_feature_list.h"
nit: empty line after #5

[hkamila, 2017-07-07 00:17:37]

Added the safe_browsing namespace and added newlines

[hkamila, 2017-07-07 00:17:55]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 00:18:10]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vakh (use Gerrit instead), 2017-07-07 00:23:41]

Looks nice overall. Happy to see JS<->C++ working :)

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/common/safe_browsing_prefs.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/common/safe_browsing_prefs.cc:461: }
This file can also be left unchanged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/features.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:7: #include <utility>
The overall structure of the file could be like this:

namespace safe_browsing {

const base::Feature
const base::Feature
const base::Feature

namespace {
  ExperimentalFeaturesList
  AddFeatureAndAvailability()
}  // namespace

  GetFeatureStatusList() or GetTrueParametersList()

}  // namespace safe_browsing

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:13: // Add your experimental features list.
Please add your features to the vector
A more descriptive comment would be useful here. Something along the lines of:

Please define any new SafeBrowsing related features in this file and add them to
the ExperimentalFeaturesList below to start displaying their status on the
chrome://safe-browsing page.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:14: // ExperimentalFeaturesListbelow as
well.
ExperimentalFeaturesList below

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:29: base::ListValue paramList;
Let's not use global variables :)

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:39: // Method function for adding the name
of the feature and deciding if
Adds the name and the enabled/disabled status of a given feature.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:52: base::ListValue GetTrueParametersList()
{
rename suggestion: GetFeatureStatusList()

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:53: for (unsigned int i = 0; i <
ExperimentalFeaturesList.size(); i++) {
why not use iterator instead?

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:54: if (ExperimentalFeaturesList[i].second
== true) {
if (ExperimentalFeaturesList[i].second)

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/features.h (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:4: #ifndef
COMPONENTS_SAFE_BROWSING_FEATURES_H_
Please leave some blank lines between the copyright comment, header, etc.
See:
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_store.cc

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/renderer/DEPS (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/renderer/DEPS:3: "+components/safe_browsing",
indentation

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/renderer/DEPS:3: "+components/safe_browsing",
would components/safe_browsing/features.h be sufficient?

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/web_ui/resources/safe_browsing.css (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/resources/safe_browsing.css:14:
background-color: #fbfbfb;
interesting choice of color :)

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/web_ui/resources/safe_browsing.js (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/resources/safe_browsing.js:15: 
indentation. Run: git cl format

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/web_ui/safe_browsing_ui.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.cc:51:
GetTrueParametersList());
Where is GetTrueParametersList defined in JS?

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.cc:53: void
SafeBrowsingUIHandler::RegisterMessages() {
Please insert a blank line between function definitions.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing_db/DEPS (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/DEPS:4: "+components/safe_browsing",
This would cause cyclic DEPS. Can we make this just:
components/safe_browsing/features.h?

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing_db/database_manager.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/database_manager.cc:3: // found in the LICENSE file.
this file can be left unchanged.

[commit-bot: I haz the power, 2017-07-07 00:46:39]

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

[commit-bot: I haz the power, 2017-07-07 00:46:41]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hkamila, 2017-07-07 01:26:41]

Added namespaces, changed function names and descriptions etc

[hkamila, 2017-07-07 01:27:12]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 01:27:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hkamila, 2017-07-07 01:29:50]

All the Acknowledged comments have been changed to match the review.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/common/safe_browsing_prefs.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/common/safe_browsing_prefs.cc:461: }
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> This file can also be left unchanged.

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/features.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:4: #include
"components/safe_browsing/features.h"
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> nit: empty line 

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:7: #include <utility>
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> The overall structure of the file could be like this:
> 
> namespace safe_browsing {
> 
> const base::Feature
> const base::Feature
> const base::Feature
> 
> namespace {
>   ExperimentalFeaturesList
>   AddFeatureAndAvailability()
> }  // namespace
> 
>   GetFeatureStatusList() or GetTrueParametersList()
> 
> }  // namespace safe_browsing

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:9: #include "base/feature_list.h"
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> nit:empty line 

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:12: 
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> safe_browsing namespace?

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:13: // Add your experimental features list.
Please add your features to the vector
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> A more descriptive comment would be useful here. Something along the lines of:
> 
> Please define any new SafeBrowsing related features in this file and add them
to
> the ExperimentalFeaturesList below to start displaying their status on the
> chrome://safe-browsing page.

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:13: // Add your experimental features list.
Please add your features to the vector
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> A more descriptive comment would be useful here. Something along the lines of:
> 
> Please define any new SafeBrowsing related features in this file and add them
to
> the ExperimentalFeaturesList below to start displaying their status on the
> chrome://safe-browsing page.

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:14: // ExperimentalFeaturesListbelow as
well.
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> ExperimentalFeaturesList below

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:29: base::ListValue paramList;
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> Let's not use global variables :)

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:39: // Method function for adding the name
of the feature and deciding if
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> Adds the name and the enabled/disabled status of a given feature.

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:52: base::ListValue GetTrueParametersList()
{
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> rename suggestion: GetFeatureStatusList()

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:53: for (unsigned int i = 0; i <
ExperimentalFeaturesList.size(); i++) {
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> why not use iterator instead?

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.cc:54: if (ExperimentalFeaturesList[i].second
== true) {
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> if (ExperimentalFeaturesList[i].second)

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/features.h (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:4: #ifndef
COMPONENTS_SAFE_BROWSING_FEATURES_H_
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> Please leave some blank lines between the copyright comment, header, etc.
> See:
> https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_store.cc

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:5: #define
COMPONENTS_SAFE_BROWSING_FEATURES_H_
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> nit: Add an empty line under #define

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:10: #include "base/feature_list.h"
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> nit: a blank line before #include "base/feature_list.h"

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/features.h:16: 
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> maybe put this class into safe_browsing namespace?

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/web_ui/safe_browsing_ui.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.cc:20: 
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> safe_browsing namespace

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.cc:51:
GetTrueParametersList());
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> Where is GetTrueParametersList defined in JS?

To my understanding, function addExperiment(result) in Javascript gets the
return value of GetTrueParametersList(), which is defined in features.cc.

(Name changed to getFeatureStatusList() )

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.cc:53: void
SafeBrowsingUIHandler::RegisterMessages() {
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> Please insert a blank line between function definitions.

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing/web_ui/safe_browsing_ui.h (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing/web_ui/safe_browsing_ui.h:18: 
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> maybe put theses 2 classes into safe_browsing namespace?

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing_db/DEPS (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/DEPS:4: "+components/safe_browsing",
On 2017/07/07 00:23:40, vakh (use Gerrit instead) wrote:
> This would cause cyclic DEPS. Can we make this just:
> components/safe_browsing/features.h?

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing_db/database_manager.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/database_manager.cc:3: // found in the LICENSE file.
On 2017/07/07 00:23:41, vakh (use Gerrit instead) wrote:
> this file can be left unchanged.

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/database_manager.cc:6: #include
"base/metrics/histogram_macros.h"
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> nit: please restore the deleted line.

Acknowledged.

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
File components/safe_browsing_db/v4_feature_list.cc (right):

https://codereview.chromium.org/2964693003/diff/140001/components/safe_browsi...
components/safe_browsing_db/v4_feature_list.cc:5: #include
"components/safe_browsing_db/v4_feature_list.h"
On 2017/07/06 23:58:27, Jialiu Lin wrote:
> nit: empty line after #5

Acknowledged.

[commit-bot: I haz the power, 2017-07-07 02:47:33]

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

[commit-bot: I haz the power, 2017-07-07 02:47:34]

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

[hkamila, 2017-07-07 16:44:59]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 16:45:13]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hkamila, 2017-07-07 17:02:43]

Fixed the copy-paste issue

[hkamila, 2017-07-07 17:02:59]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 17:03:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hkamila, 2017-07-07 17:11:11]

hkamila@google.com changed reviewers:
+ xiyuan@chromium.org

[hkamila, 2017-07-07 17:12:17]

On 2017/07/07 17:02:43, hkamila wrote:
> Fixed the copy-paste issue

@ xiyuan@chromium.org

Please review changes on
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc

[hkamila, 2017-07-07 17:26:05]

changed references for pointers

[hkamila, 2017-07-07 17:26:18]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 17:26:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hkamila, 2017-07-07 17:34:17]

On 2017/07/07 17:26:05, hkamila wrote:
> changed references for pointers

@Jialiu Lin, @vakh 
I reviewed all the comments, please take a look.

[hkamila, 2017-07-07 18:02:49]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 18:03:01]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jialiu Lin, 2017-07-07 18:10:54]

LGTM

[hkamila, 2017-07-07 18:11:40]

The CQ bit was checked by hkamila@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 18:11:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-07 18:39:47]

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

[commit-bot: I haz the power, 2017-07-07 18:39:48]

Dry run: This issue passed the CQ dry run.

[vakh (use Gerrit instead), 2017-07-07 18:42:09]

lgtm

https://codereview.chromium.org/2964693003/diff/220001/components/safe_browsi...
File components/safe_browsing/features.cc (right):

https://codereview.chromium.org/2964693003/diff/220001/components/safe_browsi...
components/safe_browsing/features.cc:61: if ((*it).second) {
Would it->second and it->first work?

https://codereview.chromium.org/2964693003/diff/220001/components/safe_browsi...
File components/safe_browsing/features.h (right):

https://codereview.chromium.org/2964693003/diff/220001/components/safe_browsi...
components/safe_browsing/features.h:29: }  // namespace safe_browsing
#endif  // COMPONENTS_SAFE_BROWSING_FEATURES_H_ goes last

https://codereview.chromium.org/2964693003/diff/220001/components/safe_browsi...
File components/safe_browsing/web_ui/resources/safe_browsing.js (right):

https://codereview.chromium.org/2964693003/diff/220001/components/safe_browsi...
components/safe_browsing/web_ui/resources/safe_browsing.js:12: function
addExperiment(result) {
This file is not indented correctly.

Try running the linter as specified here: go/jscompiler-linter

[vakh (use Gerrit instead), 2017-07-07 18:45:04]

vakh@chromium.org changed reviewers:
+ tommycli@chromium.org
- xiyuan@chromium.org

[vakh (use Gerrit instead), 2017-07-07 18:45:47]

tommycli: Please review the minor change in
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc

Thanks!

[hkamila, 2017-07-10 18:02:05]

hkamila@google.com changed reviewers:
+ pkasting@chromium.org

[hkamila, 2017-07-10 18:02:06]

pkasting@chromium.org: Please review minor changes in
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc

Thanks!

[Peter Kasting, 2017-07-10 18:27:52]

LGTM

You are welcome (encouraged, even) to TBR OWNERS on trivial mechanical changes.

[hkamila, 2017-07-10 18:51:29]

The CQ bit was checked by hkamila@google.com

[hkamila, 2017-07-10 18:51:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2964693003/#ps220001
(title: "changed references for pointers")

[commit-bot: I haz the power, 2017-07-10 18:51:45]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-10 21:00:52]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1499712689807310,
"parent_rev": "fc74af31e49db0d577943059053b9e02d08f51ee", "commit_rev":
"fdacb19a2600b6ca47c7d58752a97bc075fd89f5"}

[commit-bot: I haz the power, 2017-07-10 21:01:08]

Description was changed from

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
==========

to

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed:
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...
==========

[commit-bot: I haz the power, 2017-07-10 21:01:11]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...

[Peter Wen, 2017-07-11 16:39:46]

A revert of this CL (patchset #12 id:220001) has been created in
https://codereview.chromium.org/2974243002/ by wnwen@chromium.org.

The reason for reverting is: Introduced new static initializer:
https://crbug.com/740955.

[hkamila, 2017-07-11 18:45:55]

Returning the vector from a getter function (Fixing Revert Patch)

[hkamila, 2017-07-11 20:02:25]

On 2017/07/11 18:45:55, hkamila wrote:
> Returning the vector from a getter function (Fixing Revert Patch)

@vakh Please review the changes in features.cc
Thanks!

[hkamila, 2017-07-11 20:06:31]

Description was changed from

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed:
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...
==========

to

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed:
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...

BUG=740955
==========

[vakh (use Gerrit instead), 2017-07-11 20:25:30]

lgtm

One thing that I just noticed is that this CL is violating the style guide on
variable names: https://google.github.io/styleguide/cppguide.html#Variable_Names

Feel free to fix that in a new CL.

[hkamila, 2017-07-11 21:59:56]

Fixed the variable names according to the standard

[hkamila, 2017-07-11 22:04:14]

Fixed the variable names according to the standard 01

[hkamila, 2017-07-11 22:06:52]

Description was changed from

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed:
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...

BUG=740955
==========

to

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed:
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...

BUG=740955,734667
==========

[hkamila, 2017-07-11 22:08:49]

The CQ bit was checked by hkamila@google.com

[hkamila, 2017-07-11 22:08:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org, jialiul@chromium.org, blundell@chromium.org,
vakh@chromium.org
Link to the patchset: https://codereview.chromium.org/2964693003/#ps280001
(title: "Fixed the variable names according to the standard 01")

[commit-bot: I haz the power, 2017-07-11 22:09:42]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-12 02:50:51]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1499810929917640,
"parent_rev": "c4785f3feaa15bf042678e281f36ff2f5ad92f19", "commit_rev":
"0ee87a3ac0cc15d5c7bb3d5590bfa61620646954"}

[commit-bot: I haz the power, 2017-07-12 02:51:10]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1499810929917640,
"parent_rev": "811f6da4defd33a51bcdd2fb394ad02061af29e1", "commit_rev":
"422a878152e9cbada622e6e7f6cfc98d60af96b0"}

[commit-bot: I haz the power, 2017-07-12 02:51:27]

Description was changed from

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed:
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...

BUG=740955,734667
==========

to

==========
-Added the experimental features section as part of the chrome://safe-browsing.
For the experiments not running at 100%. 

-Exported the list of SafeBrowsing features (base::Feature) to have them in the
future in a central location. 

-Fixed the message dialogue for the WebUI.

-Added some extra css.

Note: Tags for the experiments are not added in this issue.
Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485380}
Committed:
https://chromium.googlesource.com/chromium/src/+/fdacb19a2600b6ca47c7d58752a9...

BUG=740955,734667

Review-Url: https://codereview.chromium.org/2964693003
Cr-Commit-Position: refs/heads/master@{#485821}
Committed:
https://chromium.googlesource.com/chromium/src/+/422a878152e9cbada622e6e7f6cf...
==========

[commit-bot: I haz the power, 2017-07-12 02:51:31]

Committed patchset #15 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/422a878152e9cbada622e6e7f6cf...