Skip to content
  • bsep's avatar
    Revert of Make TestRenderWidgetHostView::Show/Hide call through to RWHI... · f3b00fa5
    bsep authored
    Revert of Make TestRenderWidgetHostView::Show/Hide call through to RWHI (patchset #9 id:160001 of https://codereview.chromium.org/1713473002/ )
    
    Reason for revert:
    Speculative revert. Many tests are crashing on CrOS, yours is the only plausible CL. Apologizes in advance if it turns out to be the wrong CL.
    
    Example failure:
    https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/22356
    
    Original issue's description:
    > Make TestRenderWidgetHostView::Show/Hide call through to RWHI
    >
    > Before this patch, after calling test_web_contents->WasHidden() on a
    > TestWebContents, test_web_contents->GetMainFrame()->GetVisibilityState()
    > would still return blink::WebPageVisibilityStateVisible, because
    > TestRenderWidgetHostView::Hide didn't propagate the WasHidden to
    > RenderWidgetHostImpl (unlike all the non-test implementations of
    > RenderWidgetHostView).
    >
    > This patch fixes that. There don't seem to be significant downsides to
    > propagating the WasHidden to RWHI; it tries to send a few IPC messages,
    > but since Send is stubbed out, those are simply discarded.
    >
    > I did have to add a null check in
    > RenderWidgetHostViewBase::GetPhysicalBackingSize (called from
    > RWHI::GetResizeParams, called from RWHI::WasResized, called from
    > RWHI::WasShown), since gfx::Screen::GetScreen() sometimes returns null
    > in unit tests.
    >
    > And I had to update some of the web_contents_impl_unittest.cc tests,
    > which were expecting interstitials etc to be hidden, even though RWHI
    > generally defaults to visible (it would be possible to instead keep the
    > is_showing_ bool in TestRenderWidgetHostView, but it seems weird to
    > allow TRWHV and RWHI to disagree about whether they are visible.
    >
    > Also there's one test that manages to hit a null screen on navigation,
    > so I added a TestScreen to
    > ExtensionContextMenuModelTest.TestPageAccessSubmenu.
    >
    > BUG=577336,636953
    >
    > Review-Url: https://codereview.chromium.org/1713473002
    > Cr-Commit-Position: refs/heads/master@{#449606}
    > Committed: https://chromium.googlesource.com/chromium/src/+/1c496374204313ae4233bc19c6495c8c2b0ee660
    
    TBR=sievers@chromium.org,boliu@chromium.org,sky@chromium.org,johnme@chromium.org
    # Skipping CQ checks because original CL landed less than 1 days ago.
    NOPRESUBMIT=true
    NOTREECHECKS=true
    NOTRY=true
    BUG=577336,636953
    
    Review-Url: https://codereview.chromium.org/2684993011
    Cr-Commit-Position: refs/heads/master@{#449797}
    f3b00fa5