884006 - "Prebuilt items" and "Values" should coexist when adding items to widgets

Status: RESOLVED FIXED

Description

[Victor Porof [:vporof][:vp]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=850145#c8

Comment 1

[Victor Porof [:vporof][:vp]]

Created attachment 765310 [details] [diff] [review]
v1

Anton, would you like me to change anything else in the Profiler?

Comment 2

[Anton Kovalyov (:anton)]

Comment on attachment 765310 [details] [diff] [review]
v1

Review of attachment 765310 [details] [diff] [review]:
-----------------------------------------------------------------

I think that's chill. The code can now be refactored to have UIDs as values, instead of putting them into attachments but, I think it's better to do that in a followup. (File a bug, please?)

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Anton Kovalyov (:anton) from comment #2)
> Comment on attachment 765310 [details] [diff] [review]
> v1
> 
> Review of attachment 765310 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think that's chill. The code can now be refactored to have UIDs as values,
> instead of putting them into attachments but, I think it's better to do that
> in a followup. (File a bug, please?)

Yeah, I was thinking about doing that too, but I wanted to check with you first. Ok!

Comment 4

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 765310 [details] [diff] [review]
v1

Review of attachment 765310 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/netmonitor/netmonitor-view.js
@@ -259,5 @@
>   */
>  function RequestsMenuView() {
>    dumpn("RequestsMenuView was instantiated");
>  
> -  this._cache = new Map(); // Can't use a WeakMap because keys are strings.

didn't we just add this? :)

Comment 5

[Victor Porof [:vporof][:vp]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #4)
> Comment on attachment 765310 [details] [diff] [review]
> v1
> 
> Review of attachment 765310 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/netmonitor/netmonitor-view.js
> @@ -259,5 @@
> >   */
> >  function RequestsMenuView() {
> >    dumpn("RequestsMenuView was instantiated");
> >  
> > -  this._cache = new Map(); // Can't use a WeakMap because keys are strings.
> 
> didn't we just add this? :)

It was there since the Network Monitor inception, so .. no, it's pretty old. In the light of recent changes, it's also unnecessary and obsolete.

Comment 6

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/1d52e5f47f5a

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/1d52e5f47f5a