DevTools: render product badges behind the setting.

DevTools: render product badges behind the setting.

Review-Url: https://codereview.chromium.org/2879943003
Cr-Original-Commit-Position: refs/heads/master@{#471581}
Committed: https://chromium.googlesource.com/chromium/src/+/0bb96fac167b5266e56b7638cfe1a35a8f7fa488
Review-Url: https://codereview.chromium.org/2879943003
Cr-Commit-Position: refs/heads/master@{#471878}
Committed: https://chromium.googlesource.com/chromium/src/+/c5f38c8b3f5d08d086d2f2a4f35b004a4748665c

[pfeldman, 2017-05-12 19:14:47]

pfeldman@chromium.org changed reviewers:
+ allada@chromium.org, dgozman@chromium.org

[dgozman, 2017-05-12 23:23:56]

lgtm

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

https://codereview.chromium.org/2879943003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:62:
var parsedUrl = new Common.ParsedURL(frame.url);
Should you check parsedUrl.isValid() or does registry handle it for you?

https://codereview.chromium.org/2879943003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:68:
parsedUrl = new Common.ParsedURL(callFrame.url);
Nasty!

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

https://codereview.chromium.org/2879943003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/ProductRegistry.js:18:

nit: extra empty lines

[pfeldman, 2017-05-12 23:28:43]

All done.

[alph, 2017-05-12 23:49:08]

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

[alph, 2017-05-12 23:49:09]

lgtm

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

https://codereview.chromium.org/2879943003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:49:
if (!this._setting.get())
if (this._setting.get())
  this._renderBadge(badgeElement);

https://codereview.chromium.org/2879943003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:61:
return ProductRegistry.instance().then(registry => {
nit: await

https://codereview.chromium.org/2879943003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:154:
static _colorForId(entryName) {
can you please make it public, so others (timeline) can use it.

[allada, 2017-05-13 00:33:57]

https://codereview.chromium.org/2879943003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js
(right):

https://codereview.chromium.org/2879943003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:37:
_badgeForFrameOrUrl(urlResolver) {
Just bring in a promise here and add a symbol on the element so we don't have
memory leak issues and we will not need to do two lookups if it gets re-rendered
it'll use the resolved promise.

https://codereview.chromium.org/2879943003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:94:
var tokens = entryName.replace(/[a-z]*/g, '').split(' ');
This worked well for me:

var label = entryName.replace(/\s+(?:inc|llc)\.?$/ig,
'').replace(/^[^a-zA-Z0-9]*([a-zA-Z0-9])(?:[^A-Z]*([A-Z0-9])|([a-zA-Z0-9]?)).*$/,
'$1$2$3');

[pfeldman, 2017-05-13 00:57:45]

https://codereview.chromium.org/2879943003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js
(right):

https://codereview.chromium.org/2879943003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:37:
_badgeForFrameOrUrl(urlResolver) {
On 2017/05/13 00:33:57, allada wrote:
> Just bring in a promise here and add a symbol on the element so we don't have
> memory leak issues and we will not need to do two lookups if it gets
re-rendered
> it'll use the resolved promise.

I can't bring the promise here - it would load the module. Needs to be lazy.
Re-running the resolved promise is what I do and it costs nothing.

https://codereview.chromium.org/2879943003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/product_registry/BadgePool.js:94:
var tokens = entryName.replace(/[a-z]*/g, '').split(' ');
On 2017/05/13 00:33:57, allada wrote:
> This worked well for me:
> 
> var label = entryName.replace(/\s+(?:inc|llc)\.?$/ig,
>
'').replace(/^[^a-zA-Z0-9]*([a-zA-Z0-9])(?:[^A-Z]*([A-Z0-9])|([a-zA-Z0-9]?)).*$/,
> '$1$2$3');

I don't think it is much better than mine, so why paying the price...

[pfeldman, 2017-05-13 01:27:55]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2017-05-13 01:27:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2879943003/#ps100001
(title: "again")

[commit-bot: I haz the power, 2017-05-13 01:28:15]

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-13 02:40:33]

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

[commit-bot: I haz the power, 2017-05-13 02:40:34]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[pfeldman, 2017-05-13 06:31:59]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2017-05-13 06:32: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-05-13 08:32:18]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1494657119754150,
"parent_rev": "2db565605f2c990edb8ab8cdb6fdd9cb80daff3c", "commit_rev":
"0bb96fac167b5266e56b7638cfe1a35a8f7fa488"}

[commit-bot: I haz the power, 2017-05-13 08:32:35]

Description was changed from

==========
DevTools: render product badges behind the setting.
==========

to

==========
DevTools: render product badges behind the setting.

Review-Url: https://codereview.chromium.org/2879943003
Cr-Commit-Position: refs/heads/master@{#471581}
Committed:
https://chromium.googlesource.com/chromium/src/+/0bb96fac167b5266e56b7638cfe1...
==========

[commit-bot: I haz the power, 2017-05-13 08:32:36]

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

[findit-for-me, 2017-05-13 08:48:04]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2880913002/ by
findit-for-me@appspot.gserviceaccount.com.

The reason for reverting is: 
Findit (https://goo.gl/kROfz5) identified CL at revision 471581 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb....

[pfeldman, 2017-05-15 17:52:02]

Description was changed from

==========
DevTools: render product badges behind the setting.

Review-Url: https://codereview.chromium.org/2879943003
Cr-Commit-Position: refs/heads/master@{#471581}
Committed:
https://chromium.googlesource.com/chromium/src/+/0bb96fac167b5266e56b7638cfe1...
==========

to

==========
DevTools: render product badges behind the setting.

Review-Url: https://codereview.chromium.org/2879943003
Cr-Commit-Position: refs/heads/master@{#471581}
Committed:
https://chromium.googlesource.com/chromium/src/+/0bb96fac167b5266e56b7638cfe1...
==========

[pfeldman, 2017-05-15 17:54:04]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2017-05-15 17:54:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2879943003/#ps120001
(title: "removed module from the list.")

[commit-bot: I haz the power, 2017-05-15 17:54:59]

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-15 20:04:41]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1494870844048060,
"parent_rev": "e1186dcd515d1a52561460501faf2a5b5e97ab51", "commit_rev":
"c5f38c8b3f5d08d086d2f2a4f35b004a4748665c"}

[commit-bot: I haz the power, 2017-05-15 20:04:52]

Description was changed from

==========
DevTools: render product badges behind the setting.

Review-Url: https://codereview.chromium.org/2879943003
Cr-Commit-Position: refs/heads/master@{#471581}
Committed:
https://chromium.googlesource.com/chromium/src/+/0bb96fac167b5266e56b7638cfe1...
==========

to

==========
DevTools: render product badges behind the setting.

Review-Url: https://codereview.chromium.org/2879943003
Cr-Original-Commit-Position: refs/heads/master@{#471581}
Committed:
https://chromium.googlesource.com/chromium/src/+/0bb96fac167b5266e56b7638cfe1...
Review-Url: https://codereview.chromium.org/2879943003
Cr-Commit-Position: refs/heads/master@{#471878}
Committed:
https://chromium.googlesource.com/chromium/src/+/c5f38c8b3f5d08d086d2f2a4f35b...
==========

[commit-bot: I haz the power, 2017-05-15 20:04:53]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/c5f38c8b3f5d08d086d2f2a4f35b...