CSC/ECE 517 Fall 2015 M1501 Report CSS errors to the devtools, both stored and live

Introduction

Rust

Rust is a programming language developed by Mozilla. It is used to design concurrent and safe systems. It is a systems programming language focused on three goals: safety, speed, and concurrency. It maintains these goals without having a garbage collector, making it a useful language for a number of use cases other languages aren’t good at: embedding in other languages, programs with specific space and time requirements, and writing low-level code, like device drivers and operating systems. <ref>http://doc.rust-lang.org/nightly/book</ref>

Servo

Servo is an experimental web browser layout engine. It is developed by Mozilla and written in Rust. It provides an API for hosting the engine within other software. The Servo browser currently provides developer tools to inspect DOM, execute JavaScript remotely.

Project Description<ref>https://github.com/servo/servo/wiki/CSS-parse-error-reporting</ref>

In this project we are adding capability to expose CSS parse errors, which are essentially syntax errors through developer tools in Servo. We are using Firefox remote developer tools, which is the capability to communicate with an arbitrary distant server that implements a protocol for exposing information about its web content.

As part of the scope of this project we are using Firefox remote developer tools to inspect and debug the code in Servo. Servo, is an experimental web browser from Mozilla which is under development. This project contributes to the development of this web browser Servo.

Servo implements a very basic developer tools server. This server currently supports executing JavaScript remotely and investigating the DOM (Document Object Model) tree in the document inspector. The scope of the project is to expand those capabilities by exposing CSS parsing errors.

Program Flow

Initial steps

Choose a website compatible with Servo and attach the remote developer tools to it.
Introduce traits, methods and structure members as per the project requirement
Generate CSS error messages
Build servo again with the changes

Subsequent steps

Generate messages which communicate the errors to the script thread.
Process the messages and provide support for caching them and sending them to the devtools server if it exists.
Provide the functionality to retrieve the cached CSS error messages whenever requested.

Environment Setup<ref>https://github.com/servo/servo</ref>

Pre-requisites

On Debian-based Linuxes:

sudo apt-get install curl freeglut3-dev\
    libfreetype6-dev libgl1-mesa-dri libglib2.0-dev xorg-dev \
    gperf g++ cmake python-virtualenv python-pip \
    libssl-dev libbz2-dev libosmesa6-dev libxmu6 libxmu-dev libglu1-mesa-dev

On Fedora:

sudo dnf install curl freeglut-devel libtool gcc-c++ libXi-devel \
    freetype-devel mesa-libGL-devel glib2-devel libX11-devel libXrandr-devel gperf \
    fontconfig-devel cabextract ttmkfdir python python-virtualenv python-pip expat-devel \
    rpm-build openssl-devel cmake bzip2-devel libXcursor-devel libXmu-devel mesa-libOSMesa

Cloning servo

git clone https://github.com/servo/servo

Building Servo

cd servo
./mach build --dev
./mach run tests/html/about-mozilla.html

Requirement Analysis (includes Changes Proposed)

Steps completed in the OSS project -

Setting up ParserContext structure

define a new trait called ParseErrorReporter in components/style_traits/lib.rs with an appropriate method to report an error
add error_reporter member to ParserContext that uses this method
make log_css_error in components/style/parser.rs take an &ParserContext argument and call this method
extract the existing code from log_css_err into a new type that implements ParseErrorReporter in components/layout/layout_task.rs and pass instances of that type to any code that ends up creating a ParserContext value
find all the affected dependencies in rest of the files calling methods and structures implemented from above steps
at this point, Servo should compile and execute almost exactly as before, but where RUST_LOG=style used to expose CSS errors, now RUST_LOG=layout::layout_task will be required instead.

UML Component diagram

Steps to be followed for final project -

Setting up notifications to devtools

Add a PipelineId (from components/msg/constellation_msg.rs) member to ParserContext, to represent the source of parse errors that occur
Define a new message type in ConstellationControlMsg which contains all of the information necessary to report a CSS error (including the pipeline ID), and make this new error reporter communicate with the script thread by sending messages over a Sender<ConstellationControlMsg> value that can be obtained from the code in layout_task.rs.
Process the new message type in components/script/script_task.rs by -

 * caching each reported parse error in a vector in Document (components/script/dom/document.rs)
 * checking the devtools_wants_updates flag and sending it to the devtools server if it exists (notify_devtools for a model in script_task.rs)

Retrieve any cached parse errors for a document on request in handle_get_cached_messages in components/scripts/devtools.rs

UML Component diagram

The following diagram is not exhaustive. The dependencies will be be revealed during the implementation.

Appurtenant tasks -

Since we are working on the main servo build, the changes we make to the build usually has a cascading affect on dependent libraries and files. We are also doing the tasks of resolving the issues caused by the dependencies. So, the scope of the requirements are larger than those specified above.

Design information

MVC pattern is employed in the tool for CSS parse error reporting as shown below:

The HTML is the model which takes care of the actual data. It is the text that communicates information to the reader. CSS forms the view which sets the colors, fonts in the presentation. CSS adds visual style to the content. The same content can be presented in different ways using the content. The browser is the controller which manipulates the data through forms or Javascript. The browser combines the CSS and the HTML and renders it on the screen.

Testing the code

The correctness of the changes made are tested through 3 methodologies -

Build test

The following command on linux command line compiles and verifies if the code changes are syntactically correct.

./mach build --dev

Tidy code test

The following command on linux command line tests if the code though syntactically correct complies with the Mozilla's best practices of coding

./mach test-tidy

Logical test

After passing the above tests a pull request is made to the master branch of main servo build. The code changes will then be reviewed by our points of contact from Mozilla team. Mozilla team then verifies the correctness of the code and recommend changes if some improvements are needed.

Tasks completed for final project

Step 1

Add a PipelineId (from components/msg/constellation_msg.rs) member to ParserContext, to represent the source of parse errors that occur

A new member in parser.rs was included as suggested above (along with changes in lib.rs and Cargo.toml in same folders as parser.rs in order include the struct into the file), but it was seen that the class-name PipelineId could not be used directly to instantiate a variable.

Hence, came a suggestion by Josh to define a function to return the PipelineId member in ParseErrorReporter trait and remove the PipelineId member from ParserContext. Also came a suggestion to use the existing ParseErrorReporter member error_reporter to return the PipelineId value from trait ParseErrorReporter. Following were the exact suggestions from Josh -

 - Get rid of the new member in ParserContext
 - Add a new method to the ParseErrorReporter trait which returns a PipelineId value
 - Add a new PipelineId member to the CSSErrorReporter struct, and make the new method return that value in the ParseErrorReporter implementation for CSSErrorReporter
 - call the new method using the ParseErrorReporter member that already exists in ParserContext instead of using the PipelineId member originally described in the task

Now, to include this function in ParseErrorReporter which was defined in components/style_traits/lib.rs a crate for msg folder was declared inside this lib.rs and also include a use statement for msg::constellation_msg::PipelineId. We also had to include the path of the corresponding the msg folder in components/style_traits/Cargo.toml. But this caused the issue of cyclic dependency, since constellation_msg.rs already had included the folder components/style_traits into its lib.rs.

 - Cyclic dependency : components/style_traits/lib.rs dependent on itself

When this problem was communicated to Josh, we were suggested us to move the definition of the trait ParseErrorReporter from components/style_traits/lib.rs to components/msg/lib.rs itself

This solved the previous error but threw an error is all places where ParseErrorReporter trait was used. This was because previously all functions were using "use style_traits::ParserErrorReporter" (and corresponding changes in lib.rs and Cargo.toml) to obtain the definition of ParseErrorReporter. This declaration had just been moved to another file in previous step. About 7 different files were visited and the include statements were changed to "use msg::ParserErrorReporter" (and also make the corresponding changes to their lib.rs and Cargo.toml).

After solving the previous error, another set of errors on pipelineid showed up. In fact, there was another trait called StdoutErrorReporter which was analogous in definition to our ParseErrorReporter. Hence, all the changes made in ParseErrorReporter were also made in StdoutErrorReporter in order to maintain consistency.

After solving the previous error, another set of errors were met. Apparently, the Rust language was upgraded recently. As a result of which syntaxes of some functions calling member functions of StdoutErrorReporter had to be changed from:

 let error_reporter = box StdoutErrorReporter;          to
 letbox error_reporter  = StdoutErrorReporter {
                          pipelineid: self.pipelineid,
                          };

Also, after solving the previous error, another set of errors where were found with the same dependency as previous step in a function call present in a file which was not part of project description. The lines higlighted below were throwing an error in selector_matching.rs and also in 2 other similar files:

   lazy_static! {
   pub static ref USER_OR_USER_AGENT_STYLESHEETS: Vec<Stylesheet> = {
       let mut stylesheets = vec!();
       // FIXME: presentational-hints.css should be at author origin with zero specificity.
       //        (Does it make a difference?)
       for &filename in &["user-agent.css", "servo.css", "presentational-hints.css"] {
           match read_resource_file(&[filename]) {
               Ok(res) => {
                   let ua_stylesheet = Stylesheet::from_bytes(
                       &res,
                       Url::parse(&format!("chrome:///{:?}", filename)).unwrap(),
                       None,
                       None,
                       Origin::UserAgent,
                       box StdoutErrorReporter);
                   stylesheets.push(ua_stylesheet);
               }
               Err(..) => {
                   error!("Failed to load UA stylesheet {}!", filename);
                   process::exit(1);
               }
           }
       }
       for &(ref contents, ref url) in &opts::get().user_stylesheets {
           stylesheets.push(Stylesheet::from_bytes(
               &contents, url.clone(), None, None, Origin::User, box StdoutErrorReporter));
       }
       stylesheets
    };
  }

So, following statements were changed -

   box StdoutErrorReporter                       to
   box StdoutErrorReporter { pipelineid: PipelineId::fake_root_pipeline_id() }

Similar changes had to be made to files like report.rs and window.rs containing the following statement:

   let error_reporter = CSSErrorReporter;         to
   let error_reporter = CSSErrorReporter { pipelineid:  PipelineId::fake_root_pipeline_id() };        or
   let error_reporter = box CSSErrorReporter { pipelineid: self.pipelineid, } ;                       based on the context

Along with the changes mentioned above some more minor errors and dependencies were taken care. Final changes for step 1 is available in the following link of commit to the forked repository:

     https://github.com/GauriGNaik/servo/commit/e5021c76d8239dacaff0771b043f648a97f71c42

Changes after the code review

In layout_task.rs for the pipelineid, since an id was already available, the instantiation of pipelineid was changed from PipelineId::fake_root_pipeline_id() to id

In layout_task.rs, the position of use statement "use msg::ParseErrorReporter" was changed to fit the alphabetical order

In msg/lib.rs, the position of use statement "use cssparser::{Parser, SourcePosition}" was changed to fit the alphabetical order

In msg/lib.rs, the function call "return_pipeline" was changed to "pipeline" as per servo conventions

The change made in previous step, necessitates the function declaration in script/reporter.rs to be changed from return_pipeline to pipeline.

In dom/cssstyledeclaration.rs, the position of use statement "use msg::ParseErrorReporter" was changed to fit servo conventions on functionality

In layout/context.rs, the position of use statement "use msg::compositor_msg::LayerId" was changed to fit servo conventions on functionality

In layout/window.rs, the position of use statement "use msg::ParseErrorReporter" was changed to fit servo conventions on functionality

In dom/window.rs, for the pipelineid, since an id was already available, the instantiation of pipelineid was changed from PipelineId::fake_root_pipeline_id() to id

In selector_matching.rs also the function return_pipelineid had to be redeclared as pipeline

Some more changes to make the code tidy and follow the conventions were made at followinf files - parser.rs, selector_matching.rs and stylesheets.rs

The changes made after the first code review can be found in following link:

     https://github.com/GauriGNaik/servo/commit/d756c17f4f91f9ef1cf4d9b5c750edc29b0a5071

UML Component diagram

Step 2

Define a new message type in ConstellationControlMsg which contains all of the information necessary to report a CSS error (including the pipeline ID), and make this new error reporter communicate with the script thread by sending messages over a Sender<ConstellationControlMsg> value that can be obtained from the code in layout_task.rs

A new type CSSErrorReporting was declared in script_task/lib.rs with necessary parameters

In script_task.rs, the method handle_css_error_reporting was defined, an empty stub

Also, in script_task.rs, in impl ScriptTask, the ConstellationControlMsg::CSSErrorReporting method was mapped to self.handle_css_error_reporting

Step 3 (a)

Process the new message type in components/script/script_task.rs by: caching each reported parse error in a vector in Document (components/script/dom/document.rs)

In dom/document.rs, css_errors_store was included with all 5 parameters for the CSSErrorReporting function defined in the previous step

The previous step gave errors as some of the parameters were undefined, hence a new structure CSSError was declared in script_task.rs with all the 5 parameters declareas its members

In document.rs, the css_errors_store declaration was changed to include the structure defined above as css_errors_store: DOMRefCell<Vec<CSSError>>

In order for above operation to run successfully, the structure has to be included in script_task.rs using "use script_task::CSSError" statement

For the above use statement to work, the dependency was declared in script_traits/Cargo.toml

The member css_errors_store was defined as css_errors_store: DOMRefCell::new(vec![]) in impl PartialEq

In order to perform caching of reported parse error, a function report_css_error was declared in impl PartialEq.

The statement self.css_errors_store.borrow_mut().push(css_error) was used inside the function above to perform the actual caching

Step 3 (b)

checking the devtools_wants_updates flag and sending it to the devtools server if it exists (see notify_devtools for a model in script_task.rs)

Initially, in window.rs a function was declared to get devtools_wants_updates flag and return it

This was used in handle_css_error_reporting function in script_task.rs

When enquired, with Josh if we were in the right direction, we were suggested to use a new message variant ReportCSSError in ScriptToDevtoolsControlMsg enum which already contained a CSSError object

When, the above suggestion was implemented we faced a cyclic dependency error as CSSError was declared in script_task.rs which was already depending on devtools_traits

When enquired, Josh suggested to move CSSError to devtools_traits.rs

But, we had already tried this and knew this wouldn't work as CSSError needs to implement JSTraceable which is there again in script::dom::binding....

When again enquired, Josh suggested to get around the problem by adding an entry to the list of no_jsmanaged_fields in script/dom/bindings/trace.JS

Then, an entry was added as follows:

no_jsmanaged_fields!(JSTraceable);

But it wasn't clear how we were to include JSTraceable in devtools_traits file.

What 'use' statement should be used to use JSTraceable and not throw a cyclic dependency error

Then, as per Josh's suggestions and after some more conversations following changes were finally incorporated -

 the function handle_css_error_reporting in script_task.rs was defined to obtain a page for a given pipeline_id

a variable css_error was declared which collects the value returned by CSSError for a given pipeline_id, filename, line, column and msg

push this variable into document.report_css_error

Dependencies

The above changes had a lot of dependencies which required following changes:

in script_traits/Cargo.toml, a statement was added to declare to account for dependency on cssparser

in servo/Cargo.lock, inclusion statements were used for dependencies on cssparser

in style/Cargo.toml, a statement was added to include dependencies on components/msg

in style/lib.rs a crate was included for msg folder

in media_queries.rs use statements were included for msg::ParseErrorReporter and msg::constellation_msg::PipelineId

a function CSSErrorReporterTest was declared including the member pipelineid

in impl ParseErrorReporter for CSSErrorReporterTest; the previous declaration on error_reporter was removed and the function pipeline was defined to return the current piepelineid

in function test_media_rule the definition of the member stylesheet was changed to accounting for syntax changes in Rust language due to its recent update

similar change as mentioned in previous step was performed for the member ss in the function media_query_test

in style/stylesheets.rs a use statement for msg::constellation_msg::PipelineId was added

the definition of the member stylesheet was changed accounting for syntax changes in Rust language due to its recent update

in style/viewport.rs, use statements for msg::ParseErrorReporter and msg::constellation_msg::PipelineId were added

"

the definitions of the members error_reporter, styesheet and context was changed accounting for syntax changes in Rust language due to its recent update

The commits after step 1 till here can be found in the link below:

    https://github.com/GauriGNaik/servo/commit/5b7281ac7a7416248de7871315ad79dcb3ac2804

Changes after the code review

in script_task.rs, the declaration for the member pipeline_id was removed

few changes for alignment of the functions handle_css_error_reporting, onstellationControlMsg::CSSErrorReporting and the member css_error were to be made

dependency statement on cssparser in Cargo.toml was removed

in script.traits/lib.rs, the crate declaration for crate cssparser was removed so was use statement for cssparser::SourceLocation.

The function call for CSSErrorReporting was changed to ReportCSSError

in servo/Cargo.lock, dependency statement on cssparser was removed

in style/media_queries, in the function pipeline, return statement was changed from "return self.pipelineid" to "return PipelineId::fake_root_pipeline_id()"

in style/stylesheets.rs, use statement for msg::constellation_msg::PipelineId was removed

in style/viewport.rs, use statement for msg::constellation_msg::PipelineId was removed

The commits for changes done for code reviews on steps 2 and 3 can be found in the link below:

   https://github.com/GauriGNaik/servo/commit/110e86619c5ae9a312c914ee68e6794df03ee924

UML Component diagram

Step 4

Retrieve any cached parse errors for a document on request in handle_get_cached_messages in components/scripts/devtools.rs

[Following are not the final changes. Still, we are facing a couple of errors. We wil resolve them soon]

in devtools/lib.rs, a macro DevtoolsControlMsg::FromScript was defined which instantiates a member console_message with struct variable ConsoleMessage with members message, logLevel, filename, lineNumber and columnNumber

in devtools_traits/lib.rs, a use statement "use util::mem::HeapSizeOf" was added

the definition of the struct CSSError was moved from script_task.rs to devtools_traits/lib.rs

in devtools_traits/lib.rs, inside enum DevtoolsControlMsg, a function ReportCSSError was declared with parameters PipelineId and CSSError

in bindings/trace.rs, a use statement was added to include "devtools_traits::CSSError"

a declaration was also added for no_jsmanaged_fields!(CSSError) function

in document.rs, the use statement for script_task::CSSError was removed and use statement for devtools_traits::CSSError was added

in window.rs, a function return_devtools_flag was added to return the flag devtools_wants_updates

in script/script_task.rs, use statement for devtools_traits::CSSError was added

in function handle_css_error_reporting the datatype usize was changed to u32 as per new Rust conventions

The function document.report_css_error which was taking the parameter css_error was changed to include css_error.clone() as parameter

The function document.report_css_error was defined to send pipelineid and css_error on a devtools channel if devtools_wants_updates flag is set

in script_traits/lib.rs, in the function ReportCSSError, the datatype usize was change to u32

The commits for changes after the code review on step 3 till the changes mentioned in the previous steps can be found in the link below:

   https://github.com/GauriGNaik/servo/commit/77e8721b27be8f75363aa82dd468b30ed2718e04

UML Component diagram

References

CSC/ECE 517 Fall 2015 M1501 Report CSS errors to the devtools, both stored and live

Contents

Introduction

Rust

Servo

Project Description<ref>https://github.com/servo/servo/wiki/CSS-parse-error-reporting</ref>

Program Flow

Environment Setup<ref>https://github.com/servo/servo</ref>

Pre-requisites

Cloning servo

Building Servo

Requirement Analysis (includes Changes Proposed)

Steps completed in the OSS project -

Steps to be followed for final project -

Appurtenant tasks -

Design information

Testing the code

Build test

Tidy code test

Logical test

Tasks completed for final project

Step 1

Step 2

Step 3 (a)

Step 3 (b)

Step 4

See also

References

Navigation menu

CSC/ECE 517 Fall 2015 M1501 Report CSS errors to the devtools, both stored and live

Introduction

Rust

Servo

Project Description<ref>https://github.com/servo/servo/wiki/CSS-parse-error-reporting</ref>

Program Flow

Environment Setup<ref>https://github.com/servo/servo</ref>

Pre-requisites

Cloning servo

Building Servo

Requirement Analysis (includes Changes Proposed)

Steps completed in the OSS project -

Steps to be followed for final project -

Appurtenant tasks -

Design information

Testing the code

Build test

Tidy code test

Logical test

Tasks completed for final project

Step 1

Step 2

Step 3 (a)

Step 3 (b)

Step 4

See also

References

Navigation menu

Search