1374609 - Remove webideCli.js to avoid delaying Firefox first paint

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, enhancement, P3)

Description

[Patrick Brosset <:pbro>]

As part of Quantum/Photon, we are working on improving startup. More specifically, we care a lot about reducing the time it takes to first paint the browser window. Sometimes this means removing useless code.

A new test was added to track what components are included at startup, before the first paint: browser_startup.js (http://searchfox.org/mozilla-central/rev/2bcd258281da848311769281daf735601685de2d/browser/base/content/test/performance/browser_startup.js).

It detected webideCli.js.

If I'm not mistaken, this handles the --webide command line argument that, when you start Firefox, actually just opens WebIDE instead. This was part of an old plan of ours to make WebIDE more like an actual IDE.
This plan is obsolete, so we should remove this module (and associated build files, etc.)

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Yes, seems fine to remove!

Comment 2

[Patrick Brosset <:pbro>]

For context, this is the corresponding mailing list thread: https://groups.google.com/forum/#!topic/mozilla.dev.platform/6HFzXOb_tGQ.

If someone wants to grab this bug, this should be an easy one to fix!
Please first go through our contribution docs at http://docs.firefox-dev.tools

Then, I think it's mostly going to be about deleting code:
- deleting the webideCli file itself: http://searchfox.org/mozilla-central/source/devtools/client/webide/components/webideCli.js
- then removing references to it in other files:
  http://searchfox.org/mozilla-central/source/devtools/client/webide/components/webideComponents.manifest
  http://searchfox.org/mozilla-central/source/devtools/client/webide/components/moz.build

Comment 3

[Mohammed Yaseen Khan [:foxt7ot]]

Hi
I want to work on this one.

Comment 4

[Julian Descottes [:jdescottes]]

Thank you Mohammed! Assigning the bug to you.

On top of what Patrick said: you should be able to delete http://searchfox.org/mozilla-central/source/devtools/client/webide/components/webideComponents.manifest , because this manifest is only used to declare the webideCli.js component. Which means that in the end you can delete the whole webide/components folder. And update http://searchfox.org/mozilla-central/source/devtools/client/webide/moz.build (which references the folder).

I think you will also have to delete the references to the deleted files in http://searchfox.org/mozilla-central/source/browser/installer/package-manifest.in

Our docs are at http://docs.firefox-dev.tools in case you need pointers to get started (or you can needinfo us on this bug or ping on IRC #devtools).

Comment 5

[Mohammed Yaseen Khan [:foxt7ot]]

Attachment: Patch for the bug

Hi,
Please find attached patch.

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8880645 [details] [diff] [review]
Patch for the bug

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

Quick drive by review: the files of devtools/client/webide/components/ are still in the repository. 
They should be removed using 
> hg rm devtools/client/webide/components/

Comment 7

[Julian Descottes [:jdescottes]]

After reading the discussion on #devtools it looks like you had issues creating your patch. The documentation you found mentions a workflow that publishes your patch as a gist. To be honest,  I'm not sure why, never seen this used here :)

For information, crafting a patch with mercurial is actually pretty easy:

> hg commit -m 'Bug 1374609 - your commit message. r=reviewer'
> hg export > /path/to/your/patch

This should create a properly formatted patch that you can attach here. It requires you to commit first, but then you can amend your commit if needed. 

For in-progress reviews, attaching a diff as you did is fine though.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8880645 [details] [diff] [review]
Patch for the bug

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

Looks like you are on the right track, but some files are missing, like :jdescottes mentions.

Comment 9

[Mohammed Yaseen Khan [:foxt7ot]]

Attachment: Bug1374609.patch

Thanks julian for providing further details.

Please find patch created after commit

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8881122 [details] [diff] [review]
Bug1374609.patch

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

Great, this looks good to me.  Thanks! :)

Can you change the commit message to say "r=jryans" instead of the full email?  Then we can post this to try to see what tests say.

Comment 11

[Mohammed Yaseen Khan [:foxt7ot]]

Attachment: Patch

Hi@jryans,

Thanks for the review comments. Please find attached new patch.

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8882901 [details] [diff] [review]
Patch

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

Thanks, this looks good to me! :)

I submitted this to our try server CI system.  Assuming it looks good, we can mark it for landing by setting "checkin-needed" in the keyword field of this bug, and a sheriff will land the patch.  It will be marked FIXED once the patch merges to mozilla-central.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2cc64fe212b52a23d8ab8dfbe6a519d812bdc777

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Tests look good, marking for checkin.  Thanks again for helping out! :)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc1108e125f
Removed webide components and referneces as per issue. r=jryans

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5bc1108e125f