[WebUI] Remove needless large object literal.

[WebUI] Remove needless large object literals.

R=tbreisacher@chromium.org,estade@chromium.org
BUG=None
TEST=No needless parsing/compilation and doc still makes sense.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132745

[Tyler Breisacher (Chromium), 2012-04-17 23:09:33]

The current approach has some advantages, arguably. If it's in a comment, people
might be less likely to keep it up to date and it's nice to get the syntax
highlighting. We use this pattern in flags.js, and probably other places too, so
you might want to see what other  resources/ owners think about abandoning it.

Do we have any data that indicates this will give us a particularly significant
improvement in memory usage or speed?

[Dan Beam, 2012-04-17 23:33:56]

On 2012/04/17 23:09:33, Tyler Breisacher wrote:
> The current approach has some advantages, arguably. If it's in a comment,
people
> might be less likely to keep it up to date and it's nice to get the syntax
> highlighting. We use this pattern in flags.js, and probably other places too,
so
> you might want to see what other resources/ owners think about abandoning it.

There's nothing about a dead var that requires one to update it more than if
it's a comment.

> Do we have any data that indicates this will give us a particularly
significant
> improvement in memory usage or speed?

It may not be significant, but when you open about:plugins and type
pluginDataFormat in the developer console what do you think will happen?

Additionally, what data do you have to suggest that parsing a object literal of
arrays/objects/numbers/strings and creating them is _less_ efficient than
ignoring a block of text while searching for a * followed by a /?

[Dan Beam, 2012-04-17 23:37:56]

+estade for "another resources/ owner"'s opinion

[Tyler Breisacher (Chromium), 2012-04-17 23:40:43]

There's almost certainly some speed-up, I was just curious if we know how much.
But you're right, it's such an easy change, we might as well do it. If estade
has no objections, then lgtm.

[Dan Beam, 2012-04-17 23:55:06]

On 2012/04/17 23:40:43, Tyler Breisacher wrote:
> There's almost certainly some speed-up, I was just curious if we know how
much.
> But you're right, it's such an easy change, we might as well do it. If estade
> has no objections, then lgtm.

The main issue with this in my mind is not the non-existent parse/compilation
benefit but the confusion I encountered when seeing code that is purely for
documentation purposes but is still parsed/created/kept around. It just looked
like dead code that was mistakenly left around. I think it'd be perfectly
reasonable for somebody to simply remove this if they saw that it wasn't used
anywhere and didn't happen to see it was used for documentation (for instance,
if Coverity actually ran on JS files it'd flag this as never used I imagine,
though it's global). Additionally, neither var was directly above the function
that is called by the C++ which I found less contextually useful.

[Tyler Breisacher (Chromium), 2012-04-17 23:59:24]

Now that you mention it, I actually have a vague recollection of being confused
the first time I saw this. There's a comment to explain its purpose, but it's
still a weird unused variable. You have fully convinced me. lgtm again

[Evan Stade, 2012-04-18 02:31:58]

lgtm

[commit-bot: I haz the power, 2012-04-18 02:40:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/10083028/3001

[commit-bot: I haz the power, 2012-04-18 05:49:33]

Change committed as 132745