[Webkit-unassigned] [Bug 31204] WebInspector: RR for unit test of timeline data

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31204





--- Comment #6 from Pavel Feldman <pfeldman at chromium.org>  2009-11-06 13:24:51 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 42667 [details] [details])
> > > -    setTimeout(function() {
> > > -        if (window.layoutTestController)
> > > -            layoutTestController.showWebInspector();
> > > -        window.location.href += "?reload";
> > > -    }, 0);
> > > +    if (window.layoutTestController)
> > > +        layoutTestController.showWebInspector();
> > 
> > Sync opening of web inspector will fail on windows as far as i remember.
> > 
> > > +            }, 100); // give other scripts a chance to do work before the reload
> > 
> > 100ms is not going to work on bots. What 'other' scripts and 'work' do you
> > refer to here?
> 
> The idea was not for 100ms, but to get behind other setTimeout(xxx,0) methods I
> had setup in timline-test.js.
> 

setTimeout(xxx, 0) just queues them. No need in numbers.

> > 
> > > -    }
> > > -    evaluateInWebInspector(toInject.join("\n"), doit);
> > > +    doit();
> > 
> > It is important that doit is called after evaluateInWebInspector invoked its
> > callback.
> 
> I modified the code so that the scripts are injected before the reload.   The
> functions should have long been defined before we get to this point, or is it
> that you want to defer execution of doit() until all of onload() has finished?

When page loads, it pushes dom into the frontend. The main idea is that test
execution continues (and does doit) when frontend received that information,
made additional async calls, received response and everything settles down.
That's what you violate here.

> 
> > 
> > > +if (window.layoutTestController) {
> > > +    layoutTestController.dumpAsText();
> > > +    layoutTestController.waitUntilDone();
> > > +}
> > 
> > You have it in inspector-test.js already.
> 
> Removed.
> 
> > 
> > > +        }
> > > +        markCurrentRecordAsOverhead("onload" + (ignoreLoad ? ":ignoreLoad": ""));
> > > +    }, 1000);  // Give timer fires created in script a chance to run.  Is 1 second too long to delay?
> > > +}
> > 
> > Again, time won't help. What is the goal here?
> 
> When I write a test script that does a timer fire, I want to make sure the
> timer fire goes off before we try to pull down the timeline data.  I want to
> queue up the analysis after all the other script activity we might perform,
> like a timer fire or XHR.  I know testing XHR is going to be problematic.  What
> do you suggest I do?
> 

For timers, setTimeout(0) would queue the fires. Isn't it enough?


> > > +// We ignore initial load of the page, enable inspector and initiate reload. 
> > > +// This allows inspector controller to capture events that happen during the 
> > > +// initial page load, and startup the Timeline to capture all the records from
> > > +// loading the second page.
> > 
> > I'd like to keep reload logic in one place. We can extend inspector-test to
> > make it call into
> > frontend_setUp early in the cycle.
> 
> Why don't we have a prefix like frontendSetup_ and inject and execute all of
> those methods before reload?

Fine with me, but you would need to care about order. While explicit
frontend_setup sounds inline with unit testing in general.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.