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

From Expertiza_Wiki
Revision as of 21:30, 14 December 2015 by Pkattep (talk | contribs) (→‎Step 3 (b))
Jump to navigation Jump to search

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:

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.

Completed tasks

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

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

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)

Step 4

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

See also

References

<references/>

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2015_M1501_Report_CSS_errors_to_the_devtools,_both_stored_and_live&oldid=100473"