4 ms·

As someone looking in from outside, the Chrome codebase is a treat. Anecdote. Once during the start of my career I was asked to implement a caching layer in an

by jasim 10y ago

As someone looking in from outside, the Chrome codebase is a treat. Anecdote. Once during the start of my career I was asked to implement a caching layer in an HTTP library. I had no idea what it was, so there was some reading up to do. There were the excellent guides from mnot.net, as well as the nicely written RFC 2616. But Chrome's code was the best of all - https://github.com/adobe/chromium/blob/master/net/http/http_response_headers.cc#L990 https://github.com/adobe/chromium/blob/master/net/http/http_.... If you want to know for a fact how Chrome decides caching, that code is kind of the heart of it. Recently I needed to retrieve the "Rendered Font Name" that is available in the DevTools "Computed CSS" section. This is the name of the system font that Chrome finally picks, based on the Font-Family property. This is platform specific and so not in DOM, nor available to Chrome Extensions directly. The only way it could be done was by making an extension running in debug mode and communicates to the browser thru its remote debugger protocol. (This part of the documentation is lacking, but it is an esoteric topic anyway). The good news was that the code that does this is well encapsulated and could be easily extracted into a command line utility. For the curious: https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/Source/platform/fonts/mac/FontFamilyMatcherMac.mm#129 https://chromium.googlesource.com/chromium/src/+/master/thir... (there is a nugget about real-world software in the comments) This is, at a surface level, good code to me. Things are easy to find, there is a rhyme and rhythm to the system, and feels welcoming. The thoughts of the people who designed that system over years would be great to hear. Most writing about good code on the internet comes from an OO background, mostly wrt information systems. I wonder what people who've written these systems have to say about building and engineering complex software.

mattmanser 10y ago
I'm not a fan of the comments. There's comments that are utterly useless like: // Of course, there are other factors that can force a response to always be validated or re-fetched. Gee thanks, what might those factors be? A comment like that is worse than not commenting at all. Or the real ones that get my goat, comments which just tell you what the code that follows obviously does: // If there is no Date header, then assume that the server response was // generated at the time when we received the response. Time date_value; if (!GetDateValue(&date_value)) date_value = response_time; There's even a line (1019) where they divide by 10 but don't explain why they do it. That's what I personally would comment. The code itself is fairly easy to read (though I do wonder why they bothered using TimeDelta as it just seems to make the code more complicated, and results in confusing code like this:). return TimeDelta(); // not fresh
- squeaky-clean 10y ago
  I think the comment you mention makes perfect sense within the rest of its context. It's saying the rule for determining freshness is "response_is_fresh = (freshness_lifetime > current_age)", however that's not always the case. And looking at the code, you can see it returns true early if lifetime is equal to a new TimeDelta. The GetFreshnessLifetime function below it then covers the additional cases where it returns that. Such as the headers being set to not cache, or the expiry time being earlier than the response's time (or current time if none is provided). I think it also makes sense to assume the comment is letting us know that just because HttpResponseHeaders::RequiresValidation returns false, that doesn't mean that's the only thing that can make it require/not require a re-fetch. > There's even a line (1019) where they divide by 10 but don't explain why they do it. That's what I personally would comment. This is covered near the head of the function, line 951. Using a constant such as heuristic_scalar instead of simply using '10' would be more readable though.
- groby_b 10y ago
  While I'm not on the net team - I just mangle the UI and annoy Ben ;) - I can wager a good guess: The net team used TimeDelta because for any software project of Chromium's size strong types are crucial. If they just returned an int, you'll sooner or later see it passed through five different places, and at the end location, nobody remembers if it's ms, time ticks, seconds, or even a unix time instead of a delta. TimeDelta removes that question. It also does things that have subtle issues you might miss if you did it "by hand", like saturated adds, multiplying with an integer value while handling overflow correctly, etc. These things might be overkill for smaller project, but once you have something with hundreds of contributors, every little bit helps keep the code base sane. As for the division by 10 - look up at line 953 :) Yes, it should be a named constant. And some of the comments could certainly be better. It's a work in progress. (And if you want to help with that work, we happily accept patches!)
  - mattmanser 10y ago
    Yes, that seems like a good reason to use TimeDelta.
bengoodger 10y ago
Chrome team co-founder/engineer/etc. here. Glad you found it useful! Some components like our net stack are particularly cleanly factored. Others have more room for improvement. I would say that at the beginning of the project (2006-2008) we didn't have so much of a focus on platform design, just on shipping a browser as quickly as possible. Some of the abstractions from that era haven't stood the test of time as the project has scaled to many platforms, features etc. Over the course of time we've had various refactoring projects to try and pay down some of the technical debt. The first major one was the "content refactor" from 2011. This led to the separation of the multi-process browser shell from the UI layer, which has allowed for other chromium-derived browser apps to emerge. Today, we've observed that even this layer is a bit too complicated, so we're running more projects to try and modularize it a bit more. My mental model is that the browser is kind of like a set of system services for an ephemeral app runtime, and it's good to imagine what the APIs & separation between those things should be. To aid this we've developed a new suite of IPC tools which are way more useful than the original stuff we have used for much of the lifetime of Chrome. Anyway this kind of thing requires an ongoing investment and a set of people who thrive on the art of API design and in grungy, challenging refactoring work. I probably have many more thoughts on this topic but this'll do for right now :-)
- jasim 10y ago
  Absolutely stoked to read your response. Thank you sir. There is a dearth of quality conversations on the internet about good code in a real-world messy context, mostly because the people who're doing serious work don't have the time to talk about it. Would be a good thing if you write more. In fact you folks should be writing books!
  - bengoodger 10y ago
    One maybe interesting anecdote. May 2015, a few Chrome old timers reacting to the complexity of the code, decided we should try and build something new. This is nothing new for me personally (having worked on Netscape, Firefox and Chrome, and various false-starts along the way). We decided to design a browser based on a service-oriented architecture, using our new IPC and bindings tool (Mojo). This project was called Mandoline. We got a shell up and running that could complete some of Chrome's telemetry test suite. Performance was good. The architecture was clean. Problem was, the browser didn't do all that much. While a team of 6-7 people might have been able to build a browser 10-15 years ago, today browsers are just too complex (in feature requirements). So our options were to try and convince the Chrome team (huge org) that they should drop everything and help us build this prototype into something real (unlikely, many past examples of failure of this kind of thing - see: Gecko transition/Netscape 6), or to find a way to bring this architecture into Chrome. The first not being a real option, we settled on the latter. OK so you might ask - why labor under this delusion of building a new browser from scratch at all? Why not just stay within the confines of Chrome from the start, and look at the incremental projects that can be done to pragmatically improve things? My answer is that incrementalism should not be a destination or a goal. It's a tool that helps you get somewhere interesting. If you don't know where you're going, you're lost and incrementalism is just a delusion to trick you into thinking you're making meaningful progress. In the suffocating confines of a massive codebase, it can sometimes be hard to see the forest for the trees. It can be very valuable to step aside and try something else. Stepping aside can be creating a branch and hacking away liberally, or creating something entirely new. The other benefit of doing this is that it doesn't distract or further complicate the shipping product. But then bring the learnings back into the main line. And hopefully in your project you have leads on the main line willing to learn from such discoveries. On the Chrome team we're fortunate that we do. So this is what we're doing now. We're bringing a service-oriented architecture to Chrome. A few of us have a pretty good mental picture in our heads of what the end state looks like (roughly) and we're using incrementalism to nudge the Chromium codebase there over a few years. The value in this approach is we get to validate our ideas against all the different platforms & features Chrome supports and test it on all of Chrome's users. It means if our changes land & stick that they really are by definition "good". By the end (if there ever really is one) we will have rebuilt much of the system architecture, while shipping every 6 weeks. I was interested recently to read a similar story here about the plans in Firefox to integrate some of Servo into Gecko. The rationale was very similar. The reality is that you can't burn your user base by neglecting them while you build the massive new thing, or by expecting them to switch to something else. Instead you have to embrace the complexity & figure out how to work within it, while not giving up your dreams.
samfisher83 10y ago
The chrome code base is not easy to understand and it keeps getting changed even when it really doesn't need to. For example it used to render most of the colors here Gradient.cpp. Now its rendered in some other place and its hard to figure out exactly where.

by jasim 10y ago

mattmanser 10y ago

I'm not a fan of the comments. There's comments that are utterly useless like: // Of course, there are other factors that can force a response to always be validated or re-fetched. Gee thanks, what might those factors be? A comment like that is worse than not commenting at all. Or the real ones that get my goat, comments which just tell you what the code that follows obviously does: // If there is no Date header, then assume that the server response was // generated at the time when we received the response. Time date_value; if (!GetDateValue(&date_value)) date_value = response_time; There's even a line (1019) where they divide by 10 but don't explain why they do it. That's what I personally would comment. The code itself is fairly easy to read (though I do wonder why they bothered using TimeDelta as it just seems to make the code more complicated, and results in confusing code like this:). return TimeDelta(); // not fresh

squeaky-clean 10y ago
I think the comment you mention makes perfect sense within the rest of its context. It's saying the rule for determining freshness is "response_is_fresh = (freshness_lifetime > current_age)", however that's not always the case. And looking at the code, you can see it returns true early if lifetime is equal to a new TimeDelta. The GetFreshnessLifetime function below it then covers the additional cases where it returns that. Such as the headers being set to not cache, or the expiry time being earlier than the response's time (or current time if none is provided). I think it also makes sense to assume the comment is letting us know that just because HttpResponseHeaders::RequiresValidation returns false, that doesn't mean that's the only thing that can make it require/not require a re-fetch. > There's even a line (1019) where they divide by 10 but don't explain why they do it. That's what I personally would comment. This is covered near the head of the function, line 951. Using a constant such as heuristic_scalar instead of simply using '10' would be more readable though.
groby_b 10y ago
While I'm not on the net team - I just mangle the UI and annoy Ben ;) - I can wager a good guess: The net team used TimeDelta because for any software project of Chromium's size strong types are crucial. If they just returned an int, you'll sooner or later see it passed through five different places, and at the end location, nobody remembers if it's ms, time ticks, seconds, or even a unix time instead of a delta. TimeDelta removes that question. It also does things that have subtle issues you might miss if you did it "by hand", like saturated adds, multiplying with an integer value while handling overflow correctly, etc. These things might be overkill for smaller project, but once you have something with hundreds of contributors, every little bit helps keep the code base sane. As for the division by 10 - look up at line 953 :) Yes, it should be a named constant. And some of the comments could certainly be better. It's a work in progress. (And if you want to help with that work, we happily accept patches!)
- mattmanser 10y ago
  Yes, that seems like a good reason to use TimeDelta.

bengoodger 10y ago

Chrome team co-founder/engineer/etc. here. Glad you found it useful! Some components like our net stack are particularly cleanly factored. Others have more room for improvement. I would say that at the beginning of the project (2006-2008) we didn't have so much of a focus on platform design, just on shipping a browser as quickly as possible. Some of the abstractions from that era haven't stood the test of time as the project has scaled to many platforms, features etc. Over the course of time we've had various refactoring projects to try and pay down some of the technical debt. The first major one was the "content refactor" from 2011. This led to the separation of the multi-process browser shell from the UI layer, which has allowed for other chromium-derived browser apps to emerge. Today, we've observed that even this layer is a bit too complicated, so we're running more projects to try and modularize it a bit more. My mental model is that the browser is kind of like a set of system services for an ephemeral app runtime, and it's good to imagine what the APIs & separation between those things should be. To aid this we've developed a new suite of IPC tools which are way more useful than the original stuff we have used for much of the lifetime of Chrome. Anyway this kind of thing requires an ongoing investment and a set of people who thrive on the art of API design and in grungy, challenging refactoring work. I probably have many more thoughts on this topic but this'll do for right now :-)

jasim 10y ago
Absolutely stoked to read your response. Thank you sir. There is a dearth of quality conversations on the internet about good code in a real-world messy context, mostly because the people who're doing serious work don't have the time to talk about it. Would be a good thing if you write more. In fact you folks should be writing books!
- bengoodger 10y ago
  One maybe interesting anecdote. May 2015, a few Chrome old timers reacting to the complexity of the code, decided we should try and build something new. This is nothing new for me personally (having worked on Netscape, Firefox and Chrome, and various false-starts along the way). We decided to design a browser based on a service-oriented architecture, using our new IPC and bindings tool (Mojo). This project was called Mandoline. We got a shell up and running that could complete some of Chrome's telemetry test suite. Performance was good. The architecture was clean. Problem was, the browser didn't do all that much. While a team of 6-7 people might have been able to build a browser 10-15 years ago, today browsers are just too complex (in feature requirements). So our options were to try and convince the Chrome team (huge org) that they should drop everything and help us build this prototype into something real (unlikely, many past examples of failure of this kind of thing - see: Gecko transition/Netscape 6), or to find a way to bring this architecture into Chrome. The first not being a real option, we settled on the latter. OK so you might ask - why labor under this delusion of building a new browser from scratch at all? Why not just stay within the confines of Chrome from the start, and look at the incremental projects that can be done to pragmatically improve things? My answer is that incrementalism should not be a destination or a goal. It's a tool that helps you get somewhere interesting. If you don't know where you're going, you're lost and incrementalism is just a delusion to trick you into thinking you're making meaningful progress. In the suffocating confines of a massive codebase, it can sometimes be hard to see the forest for the trees. It can be very valuable to step aside and try something else. Stepping aside can be creating a branch and hacking away liberally, or creating something entirely new. The other benefit of doing this is that it doesn't distract or further complicate the shipping product. But then bring the learnings back into the main line. And hopefully in your project you have leads on the main line willing to learn from such discoveries. On the Chrome team we're fortunate that we do. So this is what we're doing now. We're bringing a service-oriented architecture to Chrome. A few of us have a pretty good mental picture in our heads of what the end state looks like (roughly) and we're using incrementalism to nudge the Chromium codebase there over a few years. The value in this approach is we get to validate our ideas against all the different platforms & features Chrome supports and test it on all of Chrome's users. It means if our changes land & stick that they really are by definition "good". By the end (if there ever really is one) we will have rebuilt much of the system architecture, while shipping every 6 weeks. I was interested recently to read a similar story here about the plans in Firefox to integrate some of Servo into Gecko. The rationale was very similar. The reality is that you can't burn your user base by neglecting them while you build the massive new thing, or by expecting them to switch to something else. Instead you have to embrace the complexity & figure out how to work within it, while not giving up your dreams.

samfisher83 10y ago

The chrome code base is not easy to understand and it keeps getting changed even when it really doesn't need to. For example it used to render most of the colors here Gradient.cpp. Now its rendered in some other place and its hard to figure out exactly where.