Google recently introduced a cool web-based editing tool for Chromium source code, a very stripped down version of the Willy Wonka tooling Googlers get to use for non-Chromium projects.
I’ve used this tool to submit two trivial change lists (CLs, aka PRs) to Chromium, but I was curious about whether this new feature would work for something not completely trivial, and while remaining simpler than the full Checklist for contributing to Chromium.
Let’s try it.
First, find an Available GoodFirstBug. Say, this one to remove an expired flag. Let’s search for the flag. Not too many hits:
Let’s click into the first hit to open it. Click Edit Code at the top right:
A web-based code editor opens:
Remove the &kOverlayNewLayout line and the function that references it later in the file. Use the navigation panel on the left to open the .h file corresponding and remove the declaration of the feature.
Open about_flags.cc and remove the lines referencing the flag:
Note that this will create orphan variables for the flag_descriptions that we need to go delete too. Go delete those from the flag_descriptions.cc and flag_descriptions.h files.
At this point, your Pending Changes pane contains all of the files with all of the hits that came up in the search.
If we think we’re done, we can go hit the Create Change button to actually create the change for review.
However, suspiciously, we haven’t actually found anything that respects the Feature we removed. If we instead search for OverlayNewLayout we see an additional hit appears:
Having worked on stuff like this before, I know that there’s some magical marshalling of feature values from C++ into Java-land (Android’s UI layer). So we need to go eliminate the use of the Java-side feature. Search for OVERLAY_NEW_LAYOUT.
Oof. There are 8 more files: 7 are Java and one is a similarly-named variable in a CC file. (It’s a variable in C++ code that set based on the feature’s state from the Java-side based on a resource ID. Sneaky sneaky).
Worse, one of the early hits is this one:
… So we also need to remove this function entirely and update all of its callers to not check it. Fortunately, only two more files have references, but clearing up some of those implies that we might have resources that will become unused:
When we’re done pulling all the threads, the change has grown pretty big. Click Create change to put all of our changes together into a change list:
In the dialog that appears, enter a description of the changelist and click Create:
After a minute or so, a notice will appear:
Sadly, it’s not a hyperlink, but if you look down at the bottom of the editor, you’ll find a direct link to the Chromium Code Review site, where you can find the created change. After the CL is prepared, you can add reviewers for a code review, ask anyone with Try Permissions to run your change through the bots, and otherwise follow the process for contributing to Chromium.
If you need to modify a change after you’ve created a CL, you can do so using the Edit link on the Code Review Site. Push the button:
… then use the new commands down near the file list to modify (or add) files in the CL:
A few observations from this process:
1. A lot of Chromium GoodFirstBug‘s are much more complicated than they appear. This one required a followup CL to remove 400 more lines!
2. This new web-editing workflow is great for trivial fixes, but very limited. Most importantly, if you need to use any advanced tools (e.g. source formatting, manual code generators, presubmit checks, etc), you’ll need to download your CL from the code review site to use the tools on it.
3. Multi-file changes are challenging. It would be awesome to be able to just click “Edit Code” from the Chrome Source Viewer on multiple files and have them all open in the same “in-progress” change set, but it doesn’t seem to work like that (yet?). You must manually find all files after the first one in the editor’s sidebar.
I’m excited to see how Chromium’s Edit Code feature evolves.
-Eric
PS: When making downstream changes to Microsoft Edge, our internal process is somewhat similar, but this flow is a bit less intuitive. The biggest stumbling block is that when you go to commit, AzDO will complain TF401027: You need the Git 'CreateBranch' permission. To fix this, you need to name your branch with a prefix of “user/“, e.g. user/ericlaw/remove-old-policy. You’ll likely need to find a suitable reviewer by looking at MSOwners files (~replicating git ms owners) or by looking at file history to see who else touched it. You will also need a Proof-of-Presence YubiKey token (or ask a reviewer to use theirs) to land the change.
text/plain 