Clean up DownloadAttribution Finch experiments

Clean up DownloadAttribution Finch experiments

BUG=708742
Review-Url: https://codereview.chromium.org/2821163002
Cr-Commit-Position: refs/heads/master@{#468500}
Committed: https://chromium.googlesource.com/chromium/src/+/1b455e60d47dab9b1c50e6f68cef59c0751103e0

[Jialiu Lin, 2017-04-17 19:59:08]

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

[commit-bot: I haz the power, 2017-04-17 19:59:28]

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-04-17 20:50:20]

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

[commit-bot: I haz the power, 2017-04-17 20:50:21]

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

[Jialiu Lin, 2017-04-17 23:05:37]

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

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

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-04-18 00:00:33]

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

[commit-bot: I haz the power, 2017-04-18 00:00:34]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2017-04-19 19:22:21]

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

[commit-bot: I haz the power, 2017-04-19 19:22:56]

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-04-19 20:57:48]

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

[commit-bot: I haz the power, 2017-04-19 20:57:48]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2017-04-19 21:00:30]

jialiul@chromium.org changed reviewers:
+ lpz@chromium.org

[Jialiu Lin, 2017-04-19 21:00:30]

Hi lpz@, 
Could you take a look at this CL when you have time? It is not urgent. 

Thanks,

[lpz, 2017-04-20 14:37:02]

lgtm

https://codereview.chromium.org/2821163002/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc
(right):

https://codereview.chromium.org/2821163002/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:215:
content::WebContents* web_contents) {
Feels like it might be better to keep passing Profile* instead of WebContents*.
This function is actually using a Profile, and you're only taking a WebContents
to get the Profile from it.

https://codereview.chromium.org/2821163002/diff/40001/components/safe_browsin...
File components/safe_browsing/csd.proto (right):

https://codereview.chromium.org/2821163002/diff/40001/components/safe_browsin...
components/safe_browsing/csd.proto:492: // Whether DownloadAttribution Finch
experiment is enabled for this ping.
nit: if deprecated, maybe the legacy comment can be deleted (and just replaced
with "deprecated")?

[Jialiu Lin, 2017-04-20 18:02:57]

Thanks lpz!

https://codereview.chromium.org/2821163002/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc
(right):

https://codereview.chromium.org/2821163002/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.cc:215:
content::WebContents* web_contents) {
On 2017/04/20 14:37:02, lpz wrote:
> Feels like it might be better to keep passing Profile* instead of
WebContents*.
> This function is actually using a Profile, and you're only taking a
WebContents
> to get the Profile from it.

Acknowledged.
IsEnabledAndReady is only called inside SafeBrowsingNavigationObserver. 
This is a preparation step for eventually moving SafeBrowsingNavigationObserver
class into component.

https://codereview.chromium.org/2821163002/diff/40001/components/safe_browsin...
File components/safe_browsing/csd.proto (right):

https://codereview.chromium.org/2821163002/diff/40001/components/safe_browsin...
components/safe_browsing/csd.proto:492: // Whether DownloadAttribution Finch
experiment is enabled for this ping.
On 2017/04/20 14:37:02, lpz wrote:
> nit: if deprecated, maybe the legacy comment can be deleted (and just replaced
> with "deprecated")?

Done.

[Jialiu Lin, 2017-04-28 20:14:23]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-04-28 20:14:23]

The patchset sent to the CQ was uploaded after l-g-t-m from lpz@chromium.org
Link to the patchset: https://codereview.chromium.org/2821163002/#ps60001
(title: "remove legacy comment")

[commit-bot: I haz the power, 2017-04-28 20:15:18]

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-04-28 20:20:08]

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

[commit-bot: I haz the power, 2017-04-28 20:20:09]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Jialiu Lin, 2017-04-28 20:32:49]

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

[commit-bot: I haz the power, 2017-04-28 20:33:35]

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-04-28 20:33:40]

jialiul@chromium.org changed reviewers:
+ jwd@chromium.org

[Jialiu Lin, 2017-04-28 20:33:40]

+jwd@ for owner review of testing/variations/fieldtrial_testing_config.json

Thanks!

[commit-bot: I haz the power, 2017-04-28 23:19:26]

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

[commit-bot: I haz the power, 2017-04-28 23:19:27]

Dry run: This issue passed the CQ dry run.

[jwd, 2017-05-01 20:52:06]

lgtm

[Jialiu Lin, 2017-05-01 21:08:26]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-05-01 21:08:27]

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

[commit-bot: I haz the power, 2017-05-01 21:09:10]

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-05-01 21:27:15]

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

[commit-bot: I haz the power, 2017-05-01 21:27:16]

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...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Jialiu Lin, 2017-05-01 23:02:18]

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

[commit-bot: I haz the power, 2017-05-01 23:03:36]

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-05-01 23:59:37]

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

[commit-bot: I haz the power, 2017-05-01 23:59:38]

Dry run: This issue passed the CQ dry run.

[Jialiu Lin, 2017-05-02 00:05:40]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2017-05-02 00:05:41]

The patchset sent to the CQ was uploaded after l-g-t-m from lpz@chromium.org,
jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2821163002/#ps100001
(title: "revert partial change is InEnabledAndReady()")

[commit-bot: I haz the power, 2017-05-02 00:06:11]

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-05-02 00:11:42]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493683540597190,
"parent_rev": "9d04cf08c6f0850dc580ae9416365909768c011a", "commit_rev":
"1b455e60d47dab9b1c50e6f68cef59c0751103e0"}

[commit-bot: I haz the power, 2017-05-02 00:12:44]

Description was changed from

==========
Clean up DownloadAttribution Finch experiments

BUG=708742
==========

to

==========
Clean up DownloadAttribution Finch experiments

BUG=708742

Review-Url: https://codereview.chromium.org/2821163002
Cr-Commit-Position: refs/heads/master@{#468500}
Committed:
https://chromium.googlesource.com/chromium/src/+/1b455e60d47dab9b1c50e6f68cef...
==========

[commit-bot: I haz the power, 2017-05-02 00:12:45]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/1b455e60d47dab9b1c50e6f68cef...