Closed Bug 1254814 Opened 8 years ago Closed 8 years ago

Intermittent e10s browser_markup_navigation.js | Found the doctype after pressing pageup - Got 1, expected 10

Categories

(DevTools :: Inspector, defect, P1)

Product:
DevTools
Component:
Inspector
Type:
defect

Tracking

(e10s+, firefox50 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 50
Tracking Flags:
Tracking Status
e10s + ---
firefox50 --- fixed

People

(Reporter: RyanVM, Assigned: pbro)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure, Whiteboard: [btpp-fix-later])

Attachments

(4 files)

screenshot
1.11 MB, image/png
Details
test log
99.67 KB, text/plain
Details
screenshot2.png
89.05 KB, image/png
Details
Bug_1254814_-_Synthesize_the_key_to_inspect_the_el.diff
1.63 KB, patch
pbro
: review+
Details | Diff | Splinter Review
Attached image screenshot — 
Seen while trying to green up OSX e10s tests for eventual running in CI.

https://treeherder.mozilla.org/logviewer.html#?job_id=17687152&repo=try
Attached file test log — 

    
    

    
  
Removing myself from the CC list since I am not familiar with this code and am not involved in the development of this component.

    
    

    
  
The screenshot shows that a context menu is visible in the browser, it is probably capturing the keyboard events that this test simulates, and so the test fails. It looks like the markup-view isn't getting focused. Maybe the context menu remains open since an earlier test that forgets to close it? This seems possible because the menu contains image-related items, while the element below the menu is not an image. So most probably the menu was there from before.
Priority: -- → P2
Whiteboard: [btpp-fix-later]

    
  
Summary: Intermittent OSX e10s browser_markup_navigation.js | Found the doctype after pressing pageup - Got 1, expected 10 → Intermittent e10s browser_markup_navigation.js | Found the doctype after pressing pageup - Got 1, expected 10

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Wow, this failure started to become way more frequent.
57 failures in one day, compared to 6 failures per week before.
Bumping to P1.
Priority: P2 → P1

    
    

    
  

      
      
      
      

        Attached image
          
        screenshot2.png
        — 
      

    


  
Looking at one of the more recent test logs, I can see that the browser context menu is displayed here too. So I'm fairly certain that this is caused by a prior test that does not close the browser context menu and that ends up swallowing keypress events.

    
  
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED

    
    

    
      
  

    
    

    
  
I've tried 2 different approaches:

- browser_markup_load_01.js is a test that runs before browser_markup_navigation.js and that opens the browser context menu, so there are chances that it's the one leaving the menu open. Especially because this test is the only test we have that opens the context menu and then simulates the "Q" keypress to start the inspector. Other tests like browser_inspector_initialization.js programmatically hide the menu.
So if that part fails (and this particular test fails a lot: bug 1250523), then that could affect browser_markup_navigation.js.
While looking at that test, I saw that it wasn't simulating the keypress in the right process. I don't even know how that worked at all so far.
So I tried fixing that:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a88f94a6c06&group_state=expanded
TRY seems happy enough, none of these 2 tests have failed. But from comment 12, it seems like the test was failing mostly on mozilla-inbound anyway. I don't know why, but it has failed only once in the past 7 days on TRY. So I can't be sure of this fix.

- My second approach is more of a hack, and it's basically simulating an ESC keypress right when browser_markup_navigation.js starts, to dismiss any menu that might be displayed when the test starts.
This isn't great, and really, there's no reason we'd do this only for this one test, but I wanted to try it out:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ec35a9d8dea&group_state=expanded
TRY is green too, but like before, that doesn't mean much, since the test hasn't been failing on TRY for frequently.

    
    

    
  
Note that https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1254814&startday=2016-07-04&endday=2016-07-10&tree=all shows that the test mostly failed one day of last week on mozilla-inbound.

Frequency seems to have gone back to normal after that. Maybe something landed on inbound last week that caused it to fail a lot, and was backed out.

Nevertheless, approach 1 described in comment 13 makes sense and I should land that.
If the test starts to fail again very frequently in the future, we can investigate again.

    
    

    
  
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/845645460806
Synthesize the key to inspect the element in the right process; r=testonly

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/845645460806
Status: ASSIGNED → RESOLVED
Closed: 8 years ago

          status-firefox50:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50

    
  
Product: Firefox → DevTools

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: