Background tracing: Tracing scenarios can now enable/disable Blink features, for A/B testing purpos…

Background tracing: Tracing scenarios can now enable/disable Blink features, for A/B testing purposes.

Also added a scenario name to the config, for easier filtering on the backend.

R=dsinclair,shatch
BUG=546193
Committed: https://crrev.com/1607e4b8ae91d4c9ab09c54405c468e989b96e9b
Cr-Commit-Position: refs/heads/master@{#356101}

[oystein (OOO til 10th of July), 2015-10-22 23:54:35]

Description was changed from

==========
Background tracing: Tracing scenarios can now enable/disable Blink features, for
A/B testing purposes.

R=dsinclair,shatch
BUG=546193
==========

to

==========
Background tracing: Tracing scenarios can now enable/disable Blink features, for
A/B testing purposes.

R=dsinclair,shatch
BUG=546193
==========

[oystein (OOO til 10th of July), 2015-10-22 23:55:35]

Description was changed from

==========
Background tracing: Tracing scenarios can now enable/disable Blink features, for
A/B testing purposes.

R=dsinclair,shatch
BUG=546193
==========

to

==========
Background tracing: Tracing scenarios can now enable/disable Blink features, for
A/B testing purposes.

Also added a scenario name to the config, for easier filtering on the backend.

R=dsinclair,shatch
BUG=546193
==========

[oystein (OOO til 10th of July), 2015-10-22 23:55:44]

ptal!

[fmeawad, 2015-10-23 16:02:50]

fmeawad@chromium.org changed reviewers:
+ fmeawad@chromium.org

[fmeawad, 2015-10-23 16:02:50]

Does it need to be Blink specific? As far as I understand this can work on any
boolean feature.

[chromium-reviews, 2015-10-23 16:28:02]

The switches this sets is specifically used for the Blink runtime enabled
features.

On Fri, Oct 23, 2015, 09:02  <fmeawad@chromium.org> wrote:

> Does it need to be Blink specific? As far as I understand this can work on
> any
> boolean feature.
>
> https://codereview.chromium.org/1420033003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[shatch, 2015-10-26 15:24:45]

lgtm

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_config_impl.cc (right):

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_config_impl.cc:155:
dict->GetString(kConfigEnableBlinkFeatures, &config->enable_blink_features_);
Once is probably enough.

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_manager_browsertest.cc (right):

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_manager_browsertest.cc:455:
EXPECT_TRUE(scenario_activated);
Maybe set the command line first to exercise the paths where it doesn't get
activated.

[oystein (OOO til 10th of July), 2015-10-26 18:28:39]

Thanks!

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_config_impl.cc (right):

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_config_impl.cc:155:
dict->GetString(kConfigEnableBlinkFeatures, &config->enable_blink_features_);
On 2015/10/26 15:24:45, shatch wrote:
> Once is probably enough.

Oops. But what if it's wrong the first time? :'(. Done.

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
File content/browser/tracing/background_tracing_manager_browsertest.cc (right):

https://codereview.chromium.org/1420033003/diff/1/content/browser/tracing/bac...
content/browser/tracing/background_tracing_manager_browsertest.cc:455:
EXPECT_TRUE(scenario_activated);
On 2015/10/26 15:24:45, shatch wrote:
> Maybe set the command line first to exercise the paths where it doesn't get
> activated.

Done; added second test.

[oystein (OOO til 10th of July), 2015-10-26 18:29:27]

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

[commit-bot: I haz the power, 2015-10-26 18:29:41]

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

[commit-bot: I haz the power, 2015-10-26 19:27:07]

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

[commit-bot: I haz the power, 2015-10-26 19:27:07]

Dry run: This issue passed the CQ dry run.

[oystein (OOO til 10th of July), 2015-10-26 19:59:45]

The CQ bit was checked by oysteine@chromium.org

[oystein (OOO til 10th of July), 2015-10-26 19:59:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
simonhatch@chromium.org
Link to the patchset: https://codereview.chromium.org/1420033003/#ps20001
(title: "Review fixes")

[commit-bot: I haz the power, 2015-10-26 20:00:08]

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

[commit-bot: I haz the power, 2015-10-26 20:06:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-26 20:07:17]

Patchset 2 (id:??) landed as
https://crrev.com/1607e4b8ae91d4c9ab09c54405c468e989b96e9b
Cr-Commit-Position: refs/heads/master@{#356101}

[dcaiafa, 2015-10-26 21:45:11]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1417503006/ by dcaiafa@chromium.org.

The reason for reverting is: Revert due to:

content_browsertests failure:
BackgroundTracingManagerBrowserTest.SetActiveScenarioFailsWithInvalidPreemptiveConfig

content_unittests failures:
BackgroundTracingConfigTest.PreemptiveConfigFromInvalidString
BackgroundTracingConfigTest.ReactiveConfigFromInvalidString
 
Build link:
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%2...
.