DevTools: unify Network & CPU throttling

DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx

*Note: LieFi option to come in f/u patch

BUG=728262
Review-Url: https://codereview.chromium.org/2938503002
Cr-Commit-Position: refs/heads/master@{#484438}
Committed: https://chromium.googlesource.com/chromium/src/+/9a713f878e6daf3a63106e81342b3b4b153ad2fd

[chenwilliam, 2017-06-12 23:39:13]

chenwilliam@chromium.org changed reviewers:
+ dgozman@chromium.org

[chenwilliam, 2017-06-12 23:39:14]

ptal

[chenwilliam, 2017-06-13 00:55:33]

Description was changed from

==========
DevTools: unify Network & CPU throttling

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/ADVYK
Custom Profile - http://imgur.com/a/f31UU

BUG=728262
==========

[chenwilliam, 2017-06-13 00:56:22]

@dgozman - included screenshots.

[dgozman, 2017-06-13 01:47:09]

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/devtools_compatibility.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/devtools_compatibility.js:870:
'customNetworkConditions',
Do not touch this list - it's a snapshot of settings at some revision.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeToolbar.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeToolbar.js:304:
this._showThrottlingConditionsSetting, Common.UIString('Remove mobile
throttling'),
Drop 'mobile'. It's a mobile mode anyway.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:244:
UI.inspectorView.setPanelIcon('timeline', icon);
Where did this come from? Why only timeline, if I can enable it on network as
well? Let's maybe show a single indicator somewhere?

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:296:
MobileThrottling.Events = {
MobileThrottling.ThrottlingSelector.Events

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSettingsTab.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSettingsTab.js:78:
Common.UIString('%d\xD7', conditions.cpuThrottlingRate);
'%d times' maybe? 'x' looks strange.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkConfigView.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkConfigView.js:114:
var section = this._createSection(Common.UIString('Mobile throttling'),
'network-config-throttling');
Drop mobile here as well?

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/MobileThrottling.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/MobileThrottling.js:5:
SDK.MobileThrottling = {};
Move it back to NetworkManager.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:283:
MobileThrottling.Events.CPURateChanged, this._updateShowSettingsToolbarButton,
this);
You actually want to update not only on cpu throttling, but on any non-default
value.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:317:
_createThrottlingSelect() {
Inline it.

[chenwilliam, 2017-06-13 21:46:20]

Description was changed from

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/ADVYK
Custom Profile - http://imgur.com/a/f31UU

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/ADVYK
Custom Profile - http://imgur.com/a/SQSCl

BUG=728262
==========

[chenwilliam, 2017-06-13 21:54:52]

ptal. happy to discuss in person the turtle idea :)

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/devtools_compatibility.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/devtools_compatibility.js:870:
'customNetworkConditions',
On 2017/06/13 01:47:08, dgozman wrote:
> Do not touch this list - it's a snapshot of settings at some revision.

Done.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeToolbar.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeToolbar.js:304:
this._showThrottlingConditionsSetting, Common.UIString('Remove mobile
throttling'),
On 2017/06/13 01:47:08, dgozman wrote:
> Drop 'mobile'. It's a mobile mode anyway.

Done.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:244:
UI.inspectorView.setPanelIcon('timeline', icon);
On 2017/06/13 01:47:08, dgozman wrote:
> Where did this come from? Why only timeline, if I can enable it on network as
> well? Let's maybe show a single indicator somewhere?

This is basically:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

It's a fair point on why only timeline. Today, if you flip on network throttling
in Performance panel, it only shows the icon on Network panel (it also has the
red Setting icon with the tooltip hint).

Personally, I'd like to see this be turned into a "device mode" style icon. My
semi-serious idea is a turtle icon:

1) Black turtle - not throttled
2) Hare / rabbit - Fast 3G - mid-tier phone
3) Blue turtle - Slow 3G - low-tier phone
4) Upside down blue turtle - offline.

For now though, I'd like to not block this on a significant UI change so we can
get user feedback. I'm open to suggestions on quick UI changes to make this more
clear for users.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:296:
MobileThrottling.Events = {
On 2017/06/13 01:47:08, dgozman wrote:
> MobileThrottling.ThrottlingSelector.Events

Done.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSettingsTab.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSettingsTab.js:78:
Common.UIString('%d\xD7', conditions.cpuThrottlingRate);
On 2017/06/13 01:47:08, dgozman wrote:
> '%d times' maybe? 'x' looks strange.

Yeah looks a bit weird. Done.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkConfigView.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkConfigView.js:114:
var section = this._createSection(Common.UIString('Mobile throttling'),
'network-config-throttling');
On 2017/06/13 01:47:08, dgozman wrote:
> Drop mobile here as well?

Done.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/MobileThrottling.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/MobileThrottling.js:5:
SDK.MobileThrottling = {};
On 2017/06/13 01:47:08, dgozman wrote:
> Move it back to NetworkManager.

It seemed bad to me that NetworkManager knows about cpu throttling (e.g.
SDK.NetworkManager.Conditions). I'd like to do a f/u refactoring where there's
types:

SDK.NetworkManager.Conditions which is type: {download: number, upload: number,
latency: number, title: string}

SDK.CPUThrottling.Conditions which is type: {cpuThrottlingRate: number, title:
string}

and 

SDK.MobileThrottling.Conditions is a union of those two types.

What do you think?

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:283:
MobileThrottling.Events.CPURateChanged, this._updateShowSettingsToolbarButton,
this);
On 2017/06/13 01:47:08, dgozman wrote:
> You actually want to update not only on cpu throttling, but on any non-default
> value.

So I'm tracking the network throttling changes in L256. I had bump down this CPU
event listener because I don't instantiate throttlingSelector until L281.
Doesn't feel great :S

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:317:
_createThrottlingSelect() {
On 2017/06/13 01:47:09, dgozman wrote:
> Inline it.

Done.

[dgozman, 2017-06-13 23:18:39]

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:244:
UI.inspectorView.setPanelIcon('timeline', icon);
On 2017/06/13 21:54:51, chenwilliam wrote:
> On 2017/06/13 01:47:08, dgozman wrote:
> > Where did this come from? Why only timeline, if I can enable it on network
as
> > well? Let's maybe show a single indicator somewhere?
> 
> This is basically:
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...
> 
> It's a fair point on why only timeline. Today, if you flip on network
throttling
> in Performance panel, it only shows the icon on Network panel (it also has the
> red Setting icon with the tooltip hint).
> 
> Personally, I'd like to see this be turned into a "device mode" style icon. My
> semi-serious idea is a turtle icon:
> 
> 1) Black turtle - not throttled
> 2) Hare / rabbit - Fast 3G - mid-tier phone
> 3) Blue turtle - Slow 3G - low-tier phone
> 4) Upside down blue turtle - offline.
> 
> For now though, I'd like to not block this on a significant UI change so we
can
> get user feedback. I'm open to suggestions on quick UI changes to make this
more
> clear for users.

Let's work on good UI. This feature is all about good UI.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/MobileThrottling.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/MobileThrottling.js:5:
SDK.MobileThrottling = {};
On 2017/06/13 21:54:52, chenwilliam wrote:
> On 2017/06/13 01:47:08, dgozman wrote:
> > Move it back to NetworkManager.
> 
> It seemed bad to me that NetworkManager knows about cpu throttling (e.g.
> SDK.NetworkManager.Conditions). I'd like to do a f/u refactoring where there's
> types:
> 
> SDK.NetworkManager.Conditions which is type: {download: number, upload:
number,
> latency: number, title: string}
> 
> SDK.CPUThrottling.Conditions which is type: {cpuThrottlingRate: number, title:
> string}
> 
> and 
> 
> SDK.MobileThrottling.Conditions is a union of those two types.
> 
> What do you think?

Great idea, but move MobileThrottling.Conditions to mobile_throttling module
instead. It does not have to be in SDK. Let's also do it in this patch.

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2938503002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:283:
MobileThrottling.Events.CPURateChanged, this._updateShowSettingsToolbarButton,
this);
On 2017/06/13 21:54:52, chenwilliam wrote:
> On 2017/06/13 01:47:08, dgozman wrote:
> > You actually want to update not only on cpu throttling, but on any
non-default
> > value.
> 
> So I'm tracking the network throttling changes in L256. I had bump down this
CPU
> event listener because I don't instantiate throttlingSelector until L281.
> Doesn't feel great :S

Let's have a single event and update only once. If you have a control with
event, you should not skip a layer and listen to underlying model.

[chenwilliam, 2017-06-27 00:42:17]

Description was changed from

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/ADVYK
Custom Profile - http://imgur.com/a/SQSCl

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/MQszO

BUG=728262
==========

[chenwilliam, 2017-06-27 00:52:21]

Description was changed from

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/MQszO

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/MQszO

Follow-up work:
* Add LieFi option

BUG=728262
==========

[chenwilliam, 2017-06-27 19:02:48]

Description was changed from

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/MQszO

Follow-up work:
* Add LieFi option

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx

BUG=728262
==========

[chenwilliam, 2017-06-27 22:27:08]

@dgozman, please take a look? Last CL from me before maternity leave \o/

[dgozman, 2017-06-28 22:33:09]

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:282:
* @typedef {{
Should we have network:SDK.NetworkManager.Conditions as a property instead? I
believe that's easier to maintain.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/mobile_throttling/module.json
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/module.json:11:
"settingName": "cpuThrottlingRate",
This setting should either be in SDK, or be removed completely. Since You have
CPUThrottlingManager, it should have a getter for the rate as well as an event
for when the rate changes.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:190:
this._offlineCheckboxRefForTest =
MobileThrottling.NetworkThrottlingSelector.createOfflineToolbarCheckbox();
drop RefForTest

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/NetworkManager.js (right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/NetworkManager.js:214:
SDK.NetworkManager.LieFiMobileConditions = {
Put LieFi into a separate patch (so it's easy to revert).

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:283:
this._networkThrottlingRefForTest = this._createNetworkConditionsSelect();
Just call it this._networkThrottlingSelect

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:288:
this._cpuThrottlingRefForTest =
MobileThrottling.cpuThrottlingManager.createControl();
If you create control through a manager, you should listen to it's event, not to
some setting.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:502: if
(MobileThrottling.cpuThrottlingRateSetting().get() !== 1)
MobileThrottling.cpuThrottlingManager.rate()

[chenwilliam, 2017-06-29 20:30:35]

Description was changed from

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx*

*Note: LieFi option to come in f/u patch

BUG=728262
==========

[chenwilliam, 2017-06-29 20:34:03]

ptal.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:282:
* @typedef {{
On 2017/06/28 22:33:09, dgozman wrote:
> Should we have network:SDK.NetworkManager.Conditions as a property instead? I
> believe that's easier to maintain.

I agree - done.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/mobile_throttling/module.json
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/module.json:11:
"settingName": "cpuThrottlingRate",
On 2017/06/28 22:33:09, dgozman wrote:
> This setting should either be in SDK, or be removed completely. Since You have
> CPUThrottlingManager, it should have a getter for the rate as well as an event
> for when the rate changes.

OK, removed this and switched to using the manager for communicating state.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:190:
this._offlineCheckboxRefForTest =
MobileThrottling.NetworkThrottlingSelector.createOfflineToolbarCheckbox();
On 2017/06/28 22:33:09, dgozman wrote:
> drop RefForTest

Done.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/NetworkManager.js (right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/NetworkManager.js:214:
SDK.NetworkManager.LieFiMobileConditions = {
On 2017/06/28 22:33:09, dgozman wrote:
> Put LieFi into a separate patch (so it's easy to revert).

haha ok. Done.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:283:
this._networkThrottlingRefForTest = this._createNetworkConditionsSelect();
On 2017/06/28 22:33:09, dgozman wrote:
> Just call it this._networkThrottlingSelect

Done.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:288:
this._cpuThrottlingRefForTest =
MobileThrottling.cpuThrottlingManager.createControl();
On 2017/06/28 22:33:09, dgozman wrote:
> If you create control through a manager, you should listen to it's event, not
to
> some setting.

Done.

https://codereview.chromium.org/2938503002/diff/190021/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:502: if
(MobileThrottling.cpuThrottlingRateSetting().get() !== 1)
On 2017/06/28 22:33:09, dgozman wrote:
> MobileThrottling.cpuThrottlingManager.rate()

Done.

[dgozman, 2017-06-30 00:54:16]

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js
(right):

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js:8:
MobileThrottling.CPUThrottlingManager = class extends Common.Object {
Let's rename this one to ThrottlingManager, put internal settings at it, and
turn static decorate/create controls methods into it's member methods.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js:25:
rate() {
And rename this to cpuThrottlingRate().

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js:98:
MobileThrottling.cpuThrottlingManager;
Let's make this a lazy function to avoid instantiating in Main.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:252:
MobileThrottling.trackNetworkConditions = function() {
Make this a member function of CPUThrottlingManager.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:266:
MobileThrottling.customNetworkConditionsSetting = function() {
Should this be public?

[chenwilliam, 2017-06-30 22:01:11]

ptal.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js
(right):

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js:8:
MobileThrottling.CPUThrottlingManager = class extends Common.Object {
On 2017/06/30 00:54:16, dgozman wrote:
> Let's rename this one to ThrottlingManager, put internal settings at it, and
> turn static decorate/create controls methods into it's member methods.

Done.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js:25:
rate() {
On 2017/06/30 00:54:16, dgozman wrote:
> And rename this to cpuThrottlingRate().

Done.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/CPUThrottlingManager.js:98:
MobileThrottling.cpuThrottlingManager;
On 2017/06/30 00:54:16, dgozman wrote:
> Let's make this a lazy function to avoid instantiating in Main.

Done.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:252:
MobileThrottling.trackNetworkConditions = function() {
On 2017/06/30 00:54:16, dgozman wrote:
> Make this a member function of CPUThrottlingManager.

Done.

https://codereview.chromium.org/2938503002/diff/250001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:266:
MobileThrottling.customNetworkConditionsSetting = function() {
On 2017/06/30 00:54:16, dgozman wrote:
> Should this be public?

Private now.

[dgozman, 2017-06-30 23:21:49]

Let's have Presets.js, ThrottlingManager.js, MobileThrottlingSelector.js and
NetworkThrottlingSelector.js.

lgtm

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js
(right):

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:14:
this._cpuThrottlingRates = [
Place next to other presets.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:19:
SDK.targetManager.observeModels(SDK.EmulationModel, this);
Do this in a last line.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:215:
createControl() {
createCPUThrottlingSelector

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:232:
_trackNetworkConditions() {
Let's inline it.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:239:
function networkConditionsChanged() {
arrow function ftw!

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:254:
MobileThrottling.throttlingManager = function() {
return self.singleton(MobileThrottling.ThrottlingManager);

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:214:
MobileThrottling.NetworkConditionsActionDelegate = class {
ThrottlingManager.ActionDelegate

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:331: 
stray blank line

[chenwilliam, 2017-07-05 22:33:38]

The CQ bit was checked by chenwilliam@chromium.org

[chenwilliam, 2017-07-05 22:33:38]

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

[commit-bot: I haz the power, 2017-07-05 22:34:08]

CQ is trying da patch.

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

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

Description was changed from

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx*

*Note: LieFi option to come in f/u patch

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx

*Note: LieFi option to come in f/u patch

BUG=728262
==========

[chenwilliam, 2017-07-05 22:35:32]

Landing

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js
(right):

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:14:
this._cpuThrottlingRates = [
On 2017/06/30 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> Place next to other presets.

Done.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:19:
SDK.targetManager.observeModels(SDK.EmulationModel, this);
On 2017/06/30 at 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> Do this in a last line.

Done.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:215:
createControl() {
On 2017/06/30 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> createCPUThrottlingSelector

Done.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:232:
_trackNetworkConditions() {
On 2017/06/30 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> Let's inline it.

Done.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:239:
function networkConditionsChanged() {
On 2017/06/30 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> arrow function ftw!

Done.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingManager.js:254:
MobileThrottling.throttlingManager = function() {
On 2017/06/30 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> return self.singleton(MobileThrottling.ThrottlingManager);

Done.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js
(right):

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/mobile_throttling/ThrottlingSelector.js:214:
MobileThrottling.NetworkConditionsActionDelegate = class {
On 2017/06/30 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> ThrottlingManager.ActionDelegate

Done.

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2938503002/diff/330001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:331: 
On 2017/06/30 23:21:49, dgozman_ooo (send to pfeldman) wrote:
> stray blank line

Done.

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

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

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

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_...)

[chenwilliam, 2017-07-05 23:41:56]

The CQ bit was checked by chenwilliam@chromium.org

[chenwilliam, 2017-07-05 23:41:57]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2938503002/#ps390001
(title: "update test")

[commit-bot: I haz the power, 2017-07-05 23:42:13]

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:36:36]

CQ is committing da patch.

Bot data: {"patchset_id": 390001, "attempt_start_ts": 1499298116580530,
"parent_rev": "a7d3f16ffa91d191b3c752af1a728fb86ef4a9ce", "commit_rev":
"9a713f878e6daf3a63106e81342b3b4b153ad2fd"}

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

Description was changed from

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx

*Note: LieFi option to come in f/u patch

BUG=728262
==========

to

==========
DevTools: unify Network & CPU throttling

Screenshots:
Selector - http://imgur.com/a/btqpx

*Note: LieFi option to come in f/u patch

BUG=728262

Review-Url: https://codereview.chromium.org/2938503002
Cr-Commit-Position: refs/heads/master@{#484438}
Committed:
https://chromium.googlesource.com/chromium/src/+/9a713f878e6daf3a63106e81342b...
==========

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

Committed patchset #21 (id:390001) as
https://chromium.googlesource.com/chromium/src/+/9a713f878e6daf3a63106e81342b...