948566 - HTTP cache v2: Test failure: browser/devtools/debugger/test/browser_dbg_stack-02.js | Newest frame should be selected by default in the mirrored view

Status: RESOLVED WORKSFORME

(DevTools :: Debugger, defect)

Description

[Honza Bambas (:mayhemer)]

https://tbpl.mozilla.org/?tree=Gum&rev=ec2fcd5a61ae

Regular across platforms.

Comment 1

[Honza Bambas (:mayhemer)]

Looking into it.

Comment 2

[Honza Bambas (:mayhemer)]

Definitely not an index issue, bug cache2 global issue:
https://tbpl.mozilla.org/?tree=Gum&rev=cdeeb086a430

Comment 3

[Honza Bambas (:mayhemer)]

Only difference I can see in the test log is that 'Debugger:FetchedScopes' event is delivered later (after 'Debugger:EditorSourceShown') with the new cache on.

Asking few folks involved in the test.


Victor, can you please help here?  To reproduce this 100% reliable failure, just add 'return true' as following at [1] (this forces the new cache to be used):

 // static
 bool const CacheObserver::UseNewCache()
 {
+  return true;
   switch (sUseNewCache) {
     case 0: // use the old cache backend
       return false;

...rebuild with |build binaries| and run ./mach mochitest-browser browser/devtools/debugger/test/browser_dbg_stack-02.js

I don't understand the test well, so I cannot easily find out what's wrong.  If either the test, the debugger or the cache behavior is wrong.

Thanks!


[1] http://hg.mozilla.org/mozilla-central/annotate/e7a366c1036c/netwerk/cache2/CacheObserver.cpp#l81

Comment 4

[Victor Porof [:vporof][:vp]]

I don't know what might be causing this issue here. That particular debugger test isn't doing anything special, just making sure UI elements (stack frames in the debugger) are correctly displayed, and in the correct order. "Got -1, expected 0" means something isn't selected, when it should be.

Comment 5

[Honza Bambas (:mayhemer)]

Thanks Victor.  Could you just add some explanation of how the test works?  At least in a nut shell?  Or at least when the two events 'Debugger:FetchedScopes' and 'Debugger:EditorSourceShown' are fired?  It might help.  Thanks.

Comment 6

[Honza Bambas (:mayhemer)]

Disabled on gum for now: https://hg.mozilla.org/projects/gum/rev/e8f1f9bb0b5e

Comment 7

[Victor Porof [:vporof][:vp]]

(In reply to Honza Bambas (:mayhemer) from comment #5)
> Thanks Victor.  Could you just add some explanation of how the test works? 
> At least in a nut shell?  Or at least when the two events
> 'Debugger:FetchedScopes' and 'Debugger:EditorSourceShown' are fired?  It
> might help.  Thanks.

Here's what happens, in the following order:

1. The debugger backend and frontend are initialized. The procedure for this test is identical to every other test, and is implemented here [0]. Thus, the doc_recursion-stack.html file [1] is loaded as content and the debugger is opened against it.

2. The test caches various debugger frontend elements, like the debuggee tab, the frontend container window, the stackframes (call stack) view, etc. for easier access.

3. The test calls this function [2] in the content. This will cause the content to pause, because of the "debugger statement", which functions exactly like a breakpoint. This will cause the debugger to respond appropriately.

4. After various frontend bits (like the editor text content, variables view, call stack) are populated as a consequence of the debugger pausing, the test proceeds with calling `performTest`.

5. Everything is relatively straight forward from this point onwards; the test makes sure the frontend displays the appropriate information in the call stack views, and that mouse interaction works as expected.

The debugger frontend source code is found at [3]. The entry points are [4] and [5].

"Debugger:FetchedScopes" is fired whenever a js function or block scope containing variables is retrieved by the frontend from the backend. This happens when the debugger pauses, for example and asks for environment information. "Debugger:EditorSourceShown" is fired when some source code is displayed in the debugger's text editor view. See [6] for more information about those events.

[0] http://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/test/head.js#442
[1] http://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/test/doc_recursion-stack.html
[2] http://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/test/doc_recursion-stack.html#17
[3] http://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/
[4] http://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/panel.js
[5] http://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-controller.js
[6] http://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-controller.js#19

Comment 8

[Honza Bambas (:mayhemer)]

I really don't know how to move forward with fixing this test.  More and more I believe the test or its background is wrong and it's not problem in the new cache.

Rob, can you please suggest someone to help me with this?  Victor refused to fix the (his) test.

Thanks.

Comment 9

[Rob Campbell [:rc] (:robcee)]

(In reply to Honza Bambas (:mayhemer) from comment #8)
> I really don't know how to move forward with fixing this test.  More and
> more I believe the test or its background is wrong and it's not problem in
> the new cache.

I'll get to this in a minute.

> Rob, can you please suggest someone to help me with this?  Victor refused to
> fix the (his) test.

I think Victor has explained the situation. This test is not misbehaving anywhere else (confirmed by some cursory searches). Something in the new cache behavior is causing this test to fail. The contents of the test itself are pretty innocuous, it's just testing item indexes.

With this test disabled are you seeing any other failures?

Comment 10

[Honza Bambas (:mayhemer)]

(In reply to Rob Campbell [:rc] (:robcee) from comment #9)
> With this test disabled are you seeing any other failures?

Yes, there is one left, see https://bugzilla.mozilla.org/show_bug.cgi?id=957243#c5 and all 3 following (short) comments.  In short - platform cannot count on fact that Necko sends out requests in the same order as they have been requested by upper levels.  I am not aware of any requirement Necko should do that and I know it actually doesn't, we reorder stuff regularly.  So, I'm not sure it's actually a bug or just another badly written test.

According this one bug, I think it would simpler for someone who knows debugging tools (which I don't at all) then trying to figure out what's wrong.  I only found one difference in produced logs, and that is described in comment 3:

  " Only difference I can see in the test log is that 'Debugger:FetchedScopes' event is delivered later (after 'Debugger:EditorSourceShown') with the new cache on. "

Comment 11

[Victor Porof [:vporof][:vp]]

I never refused to take a closer look at this test. I'll try to figure out what's going on.

Comment 12

[Honza Bambas (:mayhemer)]

(In reply to Victor Porof [:vp] from comment #11)
> I never refused to take a closer look at this test. I'll try to figure out
> what's going on.

Thanks Rob, I appreciate it.  (And I never said you refused to take a look at this.)

Comment 13

[Honza Bambas (:mayhemer)]

(In reply to Honza Bambas (:mayhemer) from comment #12)
> (In reply to Victor Porof [:vp] from comment #11)
> > I never refused to take a closer look at this test. I'll try to figure out
> > what's going on.
> 
> Thanks Rob, I appreciate it.  (And I never said you refused to take a look
> at this.)

Oh, my apologies.  I've exchanged names on comments, sorry for that.

Victor, thanks for looking into this.  I'm just lost and it's clear that you will be able to figure out something (not necessarily a fix, but at least a clue) around this much more efficiently then me, since I have no clue around debugging tools at all.  Comment 4 sounded like "I don't want to deal with this" so I took it as refusal to actively help.  Sorry if I overreacted.

Comment 14

[Patrick McManus [:mcmanus]]

(In reply to Honza Bambas (:mayhemer) from comment #10)
> In short - platform cannot count on fact that Necko sends
> out requests in the same order as they have been requested by upper levels. 
> I am not aware of any requirement Necko should do that and I know it
> actually doesn't, we reorder stuff regularly. 

I'll confirm that - content comes back in undefined order.. its a combination of the necko scheduler, the channel priority, the network response times, and the state of various queues at submission time. Its not deterministic, nor meant to be :)

Comment 15

[Honza Bambas (:mayhemer)]

We (necko people) believe the failures here are coming from the devtools code rather then the new cache directly, hence moving to the devtools component.

Comment 16

[Honza Bambas (:mayhemer)]

Dave, also according a private mail I've sent you, could you please help here and take a look at this test failure?  All information needed should be here.  Thanks.

Comment 17

[Honza Bambas (:mayhemer)]

This is fixed by bug 986179, so after it's fixed this bug don't need to block enabling cache2, yay!  

Still this bug should be fixed.

Comment 18

[Honza Bambas (:mayhemer)]

Fixed with bug 986179.  But the bug is still there when we do more in-memory disk-backed data caching again in the future.