English 101: Code Literacy

Code literacy: the ability to read, interpret and understand code is a new phrase that has recently been added to my vocabulary. It’s something that I practice daily as a student whether I’m tutoring at Seneca’s Learning Centre or a skimming over a main() implementation for an upcoming lab.

This week in a lecture by our Professor and Mozillian, David Humphrey, we explored code snippets in various browsers including WebKit Trac, Chromium and Mozilla DXR. It became apparent that understanding another individual’s code exposes you to the authors mindset and also reveals good or bad practices. Up until now, I was unconsciously doing just this — reading code from engineers at Mozilla or Rust to progress on bugs of mine.

In essence, by gaining more and more exposure to the world of Open Source, I’m able to learn from experienced engineers around the world for free.

Replicate

Our task was simple: test which browsers did not call the onload() event when setting the src of an img element to the same value multiple times.

Now full disclaimer, I’m not particularly strong at JavaScript which made this seemingly easy task more difficult than it needed to be. I made use of fellow classmates Michael Pierre, Phil Henning and Marco Beltempo’s code examples to get started on my own.

One thing I ran into was what seemed like an infinite loop (denoted by my Macbook Pro throttling it’s fans at 150%). With some diagnostic help from our Professor, he identified that with my current logic, an infinite loop had been formed as a result of a small misunderstanding:

Incorrect onload() logic

Setting img.src inside img.onload() created an infinite loop as setting the src would cause the image to reload which causes onload() to be called and the cycle repeats over and over again!

With a list of possible fixes presented, I chose to simply set an if else condition inside the onload() to avoid this problem:

Correct onload() logic

Test

With the simple test case ready, I was able to test it along with other classmates in the following browsers. Here’s the results gathered from all of us:

  • PASSED: Google Chrome
  • PASSED: Firefox Quantum
  • PASSED: Internet Explorer 11
  • PASSED: Opera
  • FAILED: Safari

So with all but Safari passing, I knew that we had to search the WebKit code base to figure out where the bug was. Thankfully, Marco had pointed out that the following code snippet from ImageLoader.cpp looked interesting:

Is this the bug?

My gut instinct told me that we had to look further — maybe for a specific line of code, maybe even line 354 of ImageLoader.cpp:

My suspicion

Fate so has it that just at that moment, a wild David Humphrey appeared to nudge us in the right direction:

There’s few things that I would bet any money on

And so, David and I set out on the journey with his trusty lldb debugger, webkit debug build and my instructions to steer the ship.

Analyze

At first, I thought that line 273 would be a suitable breakpoint to view the value returned by image_content_.Get():

old_image_content…hmm

But as our Professor had an older version of the debug build which wasn’t completely mapped with the one I was looking at, he pointed me to the equivalent line of code on his end — line 216 found here:

Code from David’s current build

As promised, David attached his lldb debugger to Safari using the following command:

lldb -p 7373

Where the -p argument stood for the Process ID that we were attaching the debugger to (in this case, 7373 was the Process ID for Safari on David's laptop).

LLDB doing work

A tutorial and more documentation for lldb can be found here.

With a breakpoint set at line 214 (b ImageLoader.cpp:214), he continued to execute code until that breakpoint using the c command.

After that, I simply provided my test case repository which was hosted on a gh-pages branch that made it publicly accessible for anyone to view!

The breakpoint at line 214 was triggered when the page loaded for the first time (as expected):

Breakpoint hit for the first time

But it only hits that breakpoint once, despite clicking the button to invoke the img.onload() event again.. interesting!

Same breakpoint not hit a second time

Using the bt command to visualize the 9 frames helps to see what was going on before the breakpoint was hit.

Each layer that occured before we got here

David pointed out that the bug is somewhere in the 9 layers of the stack he showed but it wasn’t where I thought it was — back to the drawing board!

Discover

I knew we were close so I started looking at other parts of the code, more specifically, the function updateFromElement():

My next suspect

I noticed that line 168 returned the current URL of the img element but it ended up not being very useful (we need to find where the src would be set, not retrieved).

Next up, I saw line 197 which referenced another thing that caught my attention:

newImage = document.cachedResourceLoader().requestImage(WTFMove(request)).value_or(nullptr);

but unfortunately David mentioned that might be used in bypassing cache storage inside Safari and wasn’t what we wanted.

But he did mention that “the load event is dispatched in ImageLoader.cpp and that something “wasn’t choosing to call it”. He also mentioned that “the dispatch stuff doesn’t get called there but only defined”. What did this all mean? Basically the event for onload() was not getting dispatched or invoked a second time for some reason!

Feeling frustrated, I took a step back to really read the code. At line 217, the if statement read:

if (newImage != oldImage) {...}

Okay — this was very important but at the time, I was fixated on figuring out what happened before it and not what happened inside it. If we got the same image as the old one, the if statement would be false and skip straight to line 261. And then it hit me:

Wait a second…

And then next thing I know…

Eureka! Found it!

So it was hiding under my nose all along — inside the if statement. The bug was that the if statement code for an old image (the same URL being passed in) was never called because it was unreachable! Boy, what a great feeling it was to find it after bumping into so many walls!

Recap

What was really cool was that David linked us when the bug was introduced and by who. It turns out that it’s been around for over 12 years!!! See for yourself here and here.

It felt like digging up a dinosaur or making another discovery — now time to delete my Slack posts so my peers have to search for it too :)

Like what you read? Give Sean Prashad a round of applause.

From a quick cheer to a standing ovation, clap to show how much you enjoyed this story.