1201700 - Create memory tool shell

Status: RESOLVED FIXED

(DevTools :: Memory, defect)

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1201700-memory-tool.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=362b80ce8c0c

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8656835 [details] [diff] [review]
1201700-memory-tool.patch

r? bgrins for the toolbox options change

Comment 3

[Tim Nguyen :ntim]

Comment on attachment 8656835 [details] [diff] [review]
1201700-memory-tool.patch

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

::: browser/devtools/definitions.js
@@ +375,5 @@
>  };
>  
> +Tools.memory = {
> +  id: "memory",
> +  ordinal: 13,

It makes more sense to put it next to the Performance tab to me. Those 2 tabs are related anyway.

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8656835 [details] [diff] [review]
1201700-memory-tool.patch

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

::: browser/devtools/framework/toolbox-options.js
@@ +229,5 @@
>      };
>  
>      // Populating the default tools lists
>      let toggleableTools = gDevTools.getDefaultTools().filter(tool => {
> +      return tool.visibilityswitch && tool.displayInOptions !== false;

I'd actually prefer a name like `hideInOptions` or `hidden` and then this condition could be `hideInOptions === true`, so that it doesn't add any confusion that addon devs might need to set this (since the current name implies you would want it to be true).

A comment for this new property should also be added in gDevTools.jsm above the registerTool function.

Comment 5

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1201700-memory-tool.patch

Made toolbox definition changes and moved memory tool adjacent to perf tools

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8656887 [details] [diff] [review]
1201700-memory-tool.patch

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

::: browser/devtools/definitions.js
@@ +287,5 @@
> +  tooltip: "Memory (keyboardshortcut)",
> +  hiddenInOptions: true,
> +
> +  isTargetSupported: function (target) {
> +    // TODO Once Fx44 lands, we should add a root trait `memoryTool`

No TODOs without a bug number

::: browser/devtools/memory/memory.xul
@@ +8,5 @@
> +<?xml-stylesheet href="chrome://browser/skin/devtools/widgets.css" type="text/css"?>
> +<?xml-stylesheet href="chrome://browser/skin/devtools/memory.css" type="text/css"?>
> +<!DOCTYPE window [
> +  <!ENTITY % debuggerDTD SYSTEM "chrome://browser/locale/devtools/performance.dtd">
> +  %debuggerDTD;

Gah no more xul... [x]html pls

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6c0281a6aee3

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6c0281a6aee3