Closed Bug 1551659 Opened 5 years ago Closed 5 years ago

"Open image in new tab" scales image to 0% (1 pixel) in Fenix

Categories

(Core :: Layout, defect, P1)

Product:
Core
Component:
Layout
Platform:
All
Android
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox67.0.1 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- fixed

People

(Reporter: cpeterson, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [geckoview:m1909])

Attachments

(2 files)

JS Stack that forces the creation of the zero-sized content viewer.
4.23 KB, text/plain
Details
Bug 1551659 - Remove MVMContext::ResizeEventFlag and related code.
47 bytes, text/x-phabricator-request
Details | Review

Steps to reproduce

  1. Go to a webpage that contains images (e.g. a Google search results page) in Fenix or R-B.
  2. Long press the image.
  3. Select "Open image in new tab" menu item.

Expected behavior

The image is opened in a new tab in normal size.

Actual behavior

The image is scaled 0% and only one pixel in size. Only after touching the address bar and back, the image is shown properly.

I can reproduce this bug in both Fenix or R-B, so I assume this is a GV bug.

This bug was originally filed in the Fenix issue tracker:
https://github.com/mozilla-mobile/fenix/issues/2312

Vesta says this bug does not need to block Fenix MVP. They will remove the "Open image in new tab" context menu item for MVP. It's a new feature that's not in Fennec, so disabling it is not a functionality regression.

status-firefox67.0.1: --- → wontfix
status-firefox68: affected → fix-optional
status-firefox69: --- → affected
Whiteboard: [geckoview:fenix:m6] → [geckoview:fenix:m7]

@ Sean, does this look like a Layout bug?

The Fenix team considers this bug a P1.

Component: General → Layout
Flags: needinfo?(svoisen)
Priority: P1 → --
Product: GeckoView → Core
Summary: "Open image in new tab" scales image to 0% (1 pixel) → "Open image in new tab" scales image to 0% (1 pixel) in Fenix

I told Sean I could take a look at this.

Flags: needinfo?(svoisen) → needinfo?(emilio)

So I cannot reproduce it on Geckoview-example, either loading the image as-is on a new session, or navigating to it. There's no "open in new tab" button.

The "Open image in new tab" button in the ReferenceBrowser reproduces this, but I'm not quite sure how to get to debug this.

James, do you know how should I go about debugging this? How does ReferenceBrowser implement this? How can I either:

  • Reproduce this on GVE (preferable) or...
  • Test Reference browser with a debug Gecko build.

Relatedly, in the image document, "open in new tab" just opens about:blank, do you want a new bug for that issue?

Flags: needinfo?(emilio) → needinfo?(snorp)

[geckoview:fenix:m7] bugs should be priority P1.

I'm editing a bunch of GeckoView bugs. If you'd like to filter all this bugmail, search and destroy emails containing this UUID:

e88a5094-0fc0-4b7c-b7c5-aef00a11dbc9

Priority: -- → P1

So I could debug this a bit more. It was kinda convoluted (I'll write about it now that I got it working).

The issue is that by the time the image loads and such, we schedule a runnable that calls ImageDocument::CheckOverflowing.

In that function, mVisibleWidth and mVisibleHeight end up zero... Here's the JS stack that causes the creation of the empty content viewer.

The native stack looks like:

(gdb) bt
#0  0xc972c4d1 in nsPresContext::SetVisibleArea(nsRect const&) (this=0xb3d9d090, r=...) at /home/emilio/src/moz/gecko-4/layout/base/nsPresContext.h:365
#1  0xcb389cea in nsDocumentViewer::InitPresentationStuff(bool) (this=0xb0352240, aDoInitialReflow=<optimized out>) at /home/emilio/src/moz/gecko-4/layout/base/nsDocumentViewer.cpp:788
#2  0xcb3899cb in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool) (this=0xb0352240, aParentWidget=0xafd39400, aState=0x0, aBounds=..., aDoCreation=<optimized out>, aNeedMakeCX=<optimized out>, aForceSetNewDocument=<optimized out>) at /home/emilio/src/moz/gecko-4/layout/base/nsDocumentViewer.cpp:994
#3  0xcb3895fe in nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) (this=0xb0352240, aParentWidget=0xafd39400, aBounds=...)
    at /home/emilio/src/moz/gecko-4/layout/base/nsDocumentViewer.cpp:739
#4  0xcbff1b43 in nsDocShell::SetupNewViewer(nsIContentViewer*) (this=0xafd39800, aNewViewer=0xb0352240) at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:8421
#5  0xcbff1529 in nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) (this=0xafd39800, aContentViewer=0xb0352240, aCommand=<optimized out>, aExtraInfo=<optimized out>)
    at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:6252
#6  0xcbff48b3 in nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIPrincipal*, nsIContentSecurityPolicy*, nsIURI*, bool, bool) (this=0xafd39800, aPrincipal=0x0, aStoragePrincipal=0x0, aCSP=0x0, aBaseURI=0x0, aTryToSaveOldPresentation=<optimized out>, aCheckPermitUnload=<optimized out>) at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:7062
#7  0xcbfdd31d in nsDocShell::EnsureContentViewer() (this=0xafd39800) at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:6900
#8  0xcbfe16ee in nsDocShell::GetDocument() (this=0xafd39800) at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:3590
#9  0xcbfe8e41 in  () at /home/emilio/src/moz/gecko-4/dom/base/nsGlobalWindowInner.h:203
#10 0xc972a124 in nsPIDOMWindowOuter::MaybeCreateDoc() (this=0xb230f140) at /home/emilio/src/moz/gecko-4/dom/base/nsGlobalWindowOuter.cpp:7876
#11 0xc91e2b78 in nsPIDOMWindowOuter::GetDoc() (this=0xb230f140) at /home/emilio/src/moz/gecko-4/dom/base/nsPIDOMWindow.h:834
#12 0xc972f527 in nsPIDOMWindowOuter::EnsureInnerWindow() (this=0xb230f140) at /home/emilio/src/moz/gecko-4/dom/base/nsPIDOMWindow.h:722
#13 0xca3e8315 in mozilla::dom::ToJSValue(JSContext*, mozilla::dom::WindowProxyHolder const&, JS::MutableHandle<JS::Value>) (aCx=0xbe00a800, aArgument=..., aValue=$JS::DoubleValue(nan))
    at /home/emilio/src/moz/gecko-4/dom/bindings/ToJSValue.cpp:81
#14 0xca3e81f2 in mozilla::dom::WrapObject(JSContext*, mozilla::dom::WindowProxyHolder const&, JS::MutableHandle<JS::Value>) (cx=0xbe00a800, p=..., rval=$JS::DoubleValue(nan))
    at /home/emilio/src/moz/gecko-4/dom/bindings/BindingUtils.cpp:1142
#15 0xc9b56d8f in mozilla::dom::ContentFrameMessageManager_Binding::get_content(JSContext*, JS::Handle<JSObject*>, mozilla::dom::ContentFrameMessageManager*, JSJitGetterCallArgs) (cx=0xbe00a800, obj=(JSObject * const) 0xb25dc300 [object ContentFrameMessageManager], self=0xafd4ba00, args=$JS::DoubleValue(nan)) at MessageManagerBinding.cpp:1879
#16 0xca3fe17b in mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) (cx=0xcf110600, argc=0, vp=0xcfe86c40)
    at /home/emilio/src/moz/gecko-4/dom/bindings/BindingUtils.cpp:3059
#17 0xcc4c6038 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (cx=0xbe00a800, native=0xca3fe065 <mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:448
#18 0xcc4b6b94 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:540
#19 0xcc4b750b in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=<optimized out>, args=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:595
#20 0xcc4b827c in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (cx=<optimized out>, fval=..., thisv=..., args=..., rval=...)
    at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:611
#21 0xcc4b827c in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (cx=0xbe00a800, thisv=$JS::DoubleValue(nan), getter=$JS::DoubleValue(nan), rval=$JS::DoubleValue(nan))
    at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:735
#22 0xcc7460c0 in GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::Shape*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) (cx=0x484ea75, obj=..., receiver=..., shape=..., vp=$JS::DoubleValue(nan)) at /home/emilio/src/moz/gecko-4/js/src/vm/NativeObject.cpp:2269
#23 0xcc7460c0 in GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::Shape*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) (cx=<optimized out>, receiver=..., obj=(js::NativeObject * const) 0xbe175500 [object ContentFrameMessageManagerPrototype] delegate, shape=0xb3ee9720, vp=$JS::DoubleValue(nan)) at /home/emilio/src/moz/gecko-4/js/src/vm/NativeObject.cpp:2321
#24 0xcc746e65 in NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) (cx=<optimized out>, obj=..., receiver=$JS::DoubleValue(nan), id={asBits = 3018706800}, nameLookup=NotNameLookup, vp=$JS::DoubleValue(nan))
    at /home/emilio/src/moz/gecko-4/js/src/vm/NativeObject.cpp:2570
#25 0xcc746997 in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) (cx=0xbe00a800, obj=(js::NativeObject * const) 0xb25dc300 [object ContentFrameMessageManager], receiver=$JS::DoubleValue(nan), id={asBits = 3018706800}, vp=$JS::DoubleValue(nan)) at /home/emilio/src/moz/gecko-4/js/src/vm/NativeObject.cpp:2607
#26 0xcc4c84ce in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>) (cx=<optimized out>, obj=(JSObject * const) 0xb25dc300 [object ContentFrameMessageManager], receiver=..., id=..., vp=...) at /home/emilio/src/moz/gecko-4/js/src/vm/ObjectOperations-inl.h:117
Python Exception <class 'gdb.error'> There is no member named flags_.: 
#27 0xcc4c84ce in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>) (cx=<optimized out>, obj=(JSObject * const) 0xb25dc300 [object ContentFrameMessageManager], receiver=$JS::DoubleValue(nan), name=, vp=$JS::DoubleValue(nan)) at /home/emilio/src/moz/gecko-4/js/src/vm/ObjectOperations-inl.h:124
Python Exception <class 'gdb.error'> There is no member named flags_.: 
#28 0xcc4bae38 in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) (cx=0xb3d9d090, v=$JS::DoubleValue(nan), name=, vp=$JS::DoubleValue(nan))
    at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:4513
#29 0xcc4a91cd in Interpret(JSContext*, js::RunState&) (cx=0xbe00a800, fp=<optimized out>, script=..., pc=<optimized out>, lval=..., vp=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:218
#30 0xcc4a91cd in Interpret(JSContext*, js::RunState&) (cx=0xbe00a800, state=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:2771
#31 0xcc4a2206 in js::RunScript(JSContext*, js::RunState&) (cx=0xbe00a800, state=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:425
---Type <return> to continue, or q <return> to quit---
#32 0xcc4b6bd4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::CONSTRUCT) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:568
#33 0xcc4b7ba4 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) (cx=<optimized out>, args=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:641
#34 0xcc4b7ec3 in js::Construct(JSContext*, JS::Handle<JS::Value>, js::AnyConstructArgs const&, JS::Handle<JS::Value>, JS::MutableHandle<JSObject*>) (cx=0xbe00a800, fval=$JS::DoubleValue(nan), args=..., newTarget=$JS::DoubleValue(nan), objp=0x0) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:696
#35 0xcc4c07a8 in js::SpreadCallOperation(JSContext*, JS::Handle<JSScript*>, unsigned char*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (cx=<optimized out>, script=0xb2dca560, pc=0xb49f81be "\246V", thisv=$JS::DoubleValue(nan), callee=$JS::DoubleValue(nan), arr=$JS::DoubleValue(nan), newTarget=$JS::DoubleValue(nan), res=$JS::DoubleValue(nan))
    at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:5099
#36 0xcc4ab7f7 in Interpret(JSContext*, js::RunState&) (cx=0x484ea75, state=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:3017
#37 0xcc4a2206 in js::RunScript(JSContext*, js::RunState&) (cx=0xbe00a800, state=...) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:425
#38 0xcc4b885f in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) (cx=<optimized out>, script=0xb3efac90, envChainArg=(JSObject &) @0xb1715880 Cannot access memory at address 0x0, newTargetValue=..., evalInFrame=AbstractFramePtr ((js::ScriptFrameIter::Data *) 0x0) = {...}, result=0xcfe88400) at /home/emilio/src/moz/gecko-4/js/src/vm/Interpreter.cpp:787
#39 0xcc51e0c4 in ExecuteInExtensibleLexicalEnvironment(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>) (cx=0x484ea75, scriptArg=..., env=(JSObject * const) 0xb1715880 [object LexicalEnvironment] delegate)
    at /home/emilio/src/moz/gecko-4/js/src/builtin/Eval.cpp:475
#40 0xcc51dc75 in js::ExecuteInFrameScriptEnvironment(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JSObject*>) (cx=0xbe00a800, objArg=(JSObject * const) 0xb25dc300 [object ContentFrameMessageManager], scriptArg=0xb3efac90, envArg=0x0) at /home/emilio/src/moz/gecko-4/js/src/builtin/Eval.cpp:509
#41 0xc96ebdf6 in nsMessageManagerScriptExecutor::LoadScriptInternal(JS::Handle<JSObject*>, nsTSubstring<char16_t> const&, bool) (this=0xafd38800, aMessageManager=(JSObject * const) 0xb25dc300 [object ContentFrameMessageManager], 
    aURL=..., aRunInUniqueScope=<optimized out>) at /home/emilio/src/moz/gecko-4/dom/base/nsFrameMessageManager.cpp:1223
#42 0xcade0894 in mozilla::dom::BrowserChild::RecvLoadRemoteScript(nsTString<char16_t> const&, bool const&) (this=0xafd38800, aURL=..., aRunInGlobalScope=@0xcfe887b8: false) at /home/emilio/src/moz/gecko-4/dom/ipc/BrowserChild.cpp:2189

I'll keep digging.

So the stack above creates an empty about: blank content viewer. There's no image document whatsoever.

What creates the actual context for the image document is this:

#0  0xc972c4d1 in nsPresContext::SetVisibleArea(nsRect const&) (this=0xb3d9e0e0, r=...) at /home/emilio/src/moz/gecko-4/layout/base/nsPresContext.h:365
#1  0xcb389cea in nsDocumentViewer::InitPresentationStuff(bool) (this=0xb2b62920, aDoInitialReflow=<optimized out>) at /home/emilio/src/moz/gecko-4/layout/base/nsDocumentViewer.cpp:788
#2  0xcb3899cb in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool) (this=0xb2b62920, aParentWidget=0xafd39400, aState=0x0, aBounds=..., aDoCreation=<optimized out>, aNeedMakeCX=<optimized out>, aForceSetNewDocument=<optimized out>) at /home/emilio/src/moz/gecko-4/layout/base/nsDocumentViewer.cpp:994
#3  0xcb3895fe in nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) (this=0xb2b62920, aParentWidget=0xafd39400, aBounds=...)
    at /home/emilio/src/moz/gecko-4/layout/base/nsDocumentViewer.cpp:739
#4  0xcbff1b43 in nsDocShell::SetupNewViewer(nsIContentViewer*) (this=0xafd39800, aNewViewer=0xb2b62920) at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:8421
#5  0xcbff1529 in nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) (this=0xafd39800, aContentViewer=0xb2b62920, aCommand=<optimized out>, aExtraInfo=<optimized out>)
    at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:6252
#6  0xcbfd7285 in nsDocShell::CreateContentViewer(nsTSubstring<char> const&, nsIRequest*, nsIStreamListener**) (this=0xafd39800, aContentType=..., aRequest=0xb338484c, aContentHandler=0xafd364d4)
    at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:8224
#7  0xcbfd6451 in nsDSURIContentListener::DoContent(nsTSubstring<char> const&, bool, nsIRequest*, nsIStreamListener**, bool*) (this=0xafd26bb0, aContentType=..., aIsContentPreferred=<optimized out>, aRequest=0xb338484c, aContentHandler=0xafd364d4, aAbortProcess=0xcfe8818f) at /home/emilio/src/moz/gecko-4/docshell/base/nsDSURIContentListener.cpp:183
#8  0xc91b87a8 in nsDocumentOpenInfo::TryContentListener(nsIURIContentListener*, nsIChannel*) (this=0xafd364c0, aListener=0xafd26bb0, aChannel=0xb338484c) at /home/emilio/src/moz/gecko-4/uriloader/base/nsURILoader.cpp:744
#9  0xc91b7a59 in nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*) (this=0xafd364c0, request=0xb338484c, aCtxt=<optimized out>) at /home/emilio/src/moz/gecko-4/uriloader/base/nsURILoader.cpp:416
#10 0xc91b7484 in nsDocumentOpenInfo::OnStartRequest(nsIRequest*) (this=0xafd364c0, request=0xb338484c) at /home/emilio/src/moz/gecko-4/uriloader/base/nsURILoader.cpp:295
#11 0xc893f6e0 in mozilla::net::HttpChannelChild::DoOnStartRequest(nsIRequest*, nsISupports*) (this=0xb3384800, aRequest=0xb338484c, aContext=<optimized out>) at /home/emilio/src/moz/gecko-4/netwerk/protocol/http/HttpChannelChild.cpp:683
#12 0xc89429eb in mozilla::net::HttpChannelChild::OnStartRequest(nsresult const&, mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, mozilla::net::ParentLoadInfoForwarderArgs const&, bool const&, bool const&, bool const&, unsigned long long const&, int const&, unsigned int const&, nsTString<char> const&, nsTString<char> const&, mozilla::net::NetAddr const&, mozilla::net::NetAddr const&, unsigned int const&, nsTString<char> const&, long long const&, bool const&, bool const&, bool const&, mozilla::net::ResourceTimingStruct const&, bool const&) (this=0xb3384800, channelStatus=@0xafd43008: nsresult::NS_OK, responseHead=..., useResponseHead=@0xafd43074: true, requestHeaders=..., loadInfoForwarder=..., isFromCache=@0xafd43076: true, isRacing=@0xafd43077: false, cacheEntryAvailable=@0xafd43078: true, cacheEntryId=@0xafd4307c: 160, cacheFetchCount=@0xafd43084: 27, cacheExpirationTime=@0xafd43088: 1565320207, cachedCharset=..., securityInfoSerialization=..., selfAddr=..., peerAddr=..., cacheKey=@0xafd4317c: 0, altDataType=..., altDataLen=@0xafd4318c: -1, deliveringAltData=@0xafd43194: false, aApplyConversion=@0xafd43075: true, aIsResolvedByTRR=@0xafd43250: false, aTiming=..., aAllRedirectsSameOrigin=@0xafd432e0: true) at /home/emilio/src/moz/gecko-4/netwerk/protocol/http/HttpChannelChild.cpp:608
#13 0xc8980ce5 in mozilla::net::StartRequestEvent::Run() (this=0xafd43000) at /home/emilio/src/moz/gecko-4/netwerk/protocol/http/HttpChannelChild.cpp:427
#14 0xc88eaa6e in mozilla::net::ChannelEventQueue::RunOrEnqueue(mozilla::net::ChannelEvent*, bool) (this=0xafd93b80, aCallback=0xafd43000, aAssertionWhenNotQueued=<optimized out>)
    at /home/emilio/src/moz/gecko-4/obj-android-emulator-debug/dist/include/mozilla/net/ChannelEventQueue.h:210
#15 0xc894252a in mozilla::net::HttpChannelChild::RecvOnStartRequest(nsresult const&, mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, mozilla::net::ParentLoadInfoForwarderArgs const&, bool const&, bool const&, bool const&, unsigned long long const&, int const&, unsigned int const&, nsTString<char> const&, nsTString<char> const&, mozilla::net::NetAddr const&, mozilla::net::NetAddr const&, short const&, unsigned int const&, nsTString<char> const&, long long const&, bool const&, bool const&, bool const&, mozilla::net::ResourceTimingStruct const&, bool const&) (this=0xb3384800, channelStatus=@0xcfe88750: nsresult::NS_OK, responseHead=..., useResponseHead=@0xcfe8874f: true, requestHeaders=..., loadInfoForwarder=..., isFromCache=@0xcfe88747: true, isRacing=@0xcfe88746: false, cacheEntryAvailable=@0xcfe88745: true, cacheEntryId=@0xcfe88738: 160, cacheFetchCount=@0xcfe88734: 27, cacheExpirationTime=@0xcfe88730: 1565320207, cachedCharset=..., securityInfoSerialization=..., selfAddr=..., peerAddr=..., redirectCount=@0xcfe8870e: 0, cacheKey=@0xcfe88708: 0, altDataType=..., altDataLen=@0xcfe886f0: -1, deliveringAltData=@0xcfe886ef: false, aApplyConversion=@0xcfe886ee: true, aIsResolvedByTRR=@0xcfe886ed: false, aTiming=..., aAllRedirectsSameOrigin=@0xcfe8865f: true) at /home/emilio/src/moz/gecko-4/netwerk/protocol/http/HttpChannelChild.cpp:489
#16 0xc8cfd062 in mozilla::net::PHttpChannelChild::OnMessageReceived(IPC::Message const&) (this=0xb3384800, msg__=...) at PHttpChannelChild.cpp:859
#17 0xc8c2ac44 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) (this=0xcf12d810, msg__=...) at PContentChild.cpp:7839
#18 0xc8b2fd95 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) (this=0xcf12d89c, aProxy=0xcf10a4a0, aMsg=...) at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:2184
#19 0xc8b2ed54 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) (this=0xcf12d89c, aMsg=<unknown type in /home/emilio/src/moz/gecko-4/obj-android-emulator-debug/toolkit/library/libxul.so, CU 0x2255ff9, DIE 0x22cc2c5>)
    at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:2108
#20 0xc8b2f38e in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) (this=0xcf12d89c, aTask=...) at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:1955
#21 0xc8b2f791 in mozilla::ipc::MessageChannel::MessageTask::Run() (this=0xafd58a00) at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:1986
#22 0xc84f90b8 in mozilla::SchedulerGroup::Runnable::Run() (this=0xafd5b7c0) at /home/emilio/src/moz/gecko-4/xpcom/threads/SchedulerGroup.cpp:295
#23 0xc850e422 in nsThread::ProcessNextEvent(bool, bool*) (this=0xcf1280c0, aMayWait=<optimized out>, aResult=0xcfe89b47) at /home/emilio/src/moz/gecko-4/xpcom/threads/nsThread.cpp:1225
#24 0xc8510390 in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0xcf1280c0, aMayWait=<optimized out>) at /home/emilio/src/moz/gecko-4/xpcom/threads/nsThreadUtils.cpp:486
#25 0xc8b31e7b in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0xcf109250, aDelegate=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/glue/MessagePump.cpp:88
#26 0xc8ab8517 in MessageLoop::RunInternal() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:315
#27 0xc8ab8485 in MessageLoop::Run() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:308
#28 0xc8ab8485 in MessageLoop::Run() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:290
#29 0xcb12d735 in nsBaseAppShell::Run() (this=0xcf12bb80) at /home/emilio/src/moz/gecko-4/widget/nsBaseAppShell.cpp:137
#30 0xcc3600c3 in XRE_RunAppShell() () at /home/emilio/src/moz/gecko-4/toolkit/xre/nsEmbedFunctions.cpp:919
#31 0xc8b3258e in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (this=0xcf109250, aDelegate=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/glue/MessagePump.cpp:238
#32 0xc8ab8517 in MessageLoop::RunInternal() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:315
#33 0xc8ab8485 in MessageLoop::Run() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:308
#34 0xc8ab8485 in MessageLoop::Run() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:290

Still zero-sized.

So the visible area is 0x0, so we go to ShrinkToFit() and end up with width=1 height=1 on the image.

Then we get the document load event, and handle it in ImageDocument::HandleEvent -> UpdateSizeFromLayout. Nothing interesting yet, as viewport is still zero-sized, and actual layout width is the same as intrinsic size.

Then we switch to the tab, and finally get a size from an IPC message:

#0  0xc972c4d1 in nsPresContext::SetVisibleArea(nsRect const&) (this=0xb3d9e0e0, r=...) at /home/emilio/src/moz/gecko-4/layout/base/nsPresContext.h:365
#1  0xcb33372d in mozilla::PresShell::ResizeReflowIgnoreOverride(int, int, int, int, mozilla::ResizeReflowOptions) (this=0xb2649000, aWidth=24000, aHeight=36960, aOldWidth=0, aOldHeight=0, aOptions=mozilla::ResizeReflowOptions::SuppressRe
sizeEvent) at /home/emilio/src/moz/gecko-4/layout/base/PresShell.cpp:1918
#2  0xcb333565 in mozilla::GeckoMVMContext::Reflow(mozilla::gfx::SizeTyped<mozilla::CSSPixel, float> const&, mozilla::gfx::SizeTyped<mozilla::CSSPixel, float> const&, mozilla::MVMContext::ResizeEventFlag) (this=0xafd9dac0, aNewSize=..., a
OldSize=..., aResizeEventFlag=mozilla::MVMContext::ResizeEventFlag::Suppress) at /home/emilio/src/moz/gecko-4/layout/base/GeckoMVMContext.cpp:174
#3  0xcb3351e7 in MobileViewportManager::RefreshViewportSize(bool) (this=0xafd74d40, aForceAdjustResolution=<optimized out>) at /home/emilio/src/moz/gecko-4/layout/base/MobileViewportManager.cpp:575
#4  0xcb33bbfc in mozilla::PresShell::ResizeReflow(int, int, int, int, mozilla::ResizeReflowOptions) (this=0x484ea75, aForceAdjustResolution=<optimized out>) at /home/emilio/src/moz/gecko-4/layout/base/MobileViewportManager.cpp:105
#5  0xcb33bbfc in mozilla::PresShell::ResizeReflow(int, int, int, int, mozilla::ResizeReflowOptions) (this=0xb2649000, aWidth=24000, aHeight=36960, aOldWidth=0, aOldHeight=0, aOptions=mozilla::ResizeReflowOptions::NoOption)
    at /home/emilio/src/moz/gecko-4/layout/base/PresShell.cpp:1849
#6  0xcb0fb4ef in nsViewManager::DoSetWindowDimensions(int, int) (this=0xafd5beb0, aWidth=24000, aHeight=36960) at /home/emilio/src/moz/gecko-4/view/nsViewManager.cpp:182
#7  0xcb38e075 in nsDocumentViewer::SetBoundsWithFlags(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, unsigned int) (this=0xb2b62920, aBounds=..., aFlags=0)
    at /home/emilio/src/moz/gecko-4/layout/base/nsDocumentViewer.cpp:2139
#8  0xcbfedd34 in nsDocShell::SetPositionAndSize(int, int, int, int, unsigned int) (this=0xafd39800, aX=0, aY=0, aWidth=800, aHeight=1232, aFlags=1) at /home/emilio/src/moz/gecko-4/docshell/base/nsDocShell.cpp:5067
#9  0xcbfee8c9 in  () at /home/emilio/src/moz/gecko-4/dom/base/nsGlobalWindowInner.h:203
#10 0xcc1bca1c in nsWebBrowser::SetPositionAndSize(int, int, int, int, unsigned int) (this=0xb2c16660, aX=0, aY=0, aCX=800, aCY=1232, aFlags=1) 
#12 0xcaddce3a in mozilla::dom::BrowserChild::RecvUpdateDimensions(mozilla::dom::DimensionInfo const&) (this=0xafd38800, aDimensionInfo=...) at /home/emilio/src/moz/gecko-4/dom/ipc/BrowserChild.cpp:1253
#13 0xc8f30bed in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) (this=0xafd38820, msg__=...) at PBrowserChild.cpp:4564
#14 0xc8c2ac44 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) (this=0xcf12d810, msg__=...) at PContentChild.cpp:7839
#15 0xc8b2fd95 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) (this=0xcf12d89c, aProxy=0xcf10a4a0, aMsg=...) at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:2184
#16 0xc8b2ed54 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) (this=0xcf12d89c, aMsg=<unknown type in /home/emilio/src/moz/gecko-4/obj-android-emulator-debug/toolkit/library/libxul.so, CU 0x2255ff9, DIE 0x22cc2c5>)
    at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:2108
#17 0xc8b2f38e in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) (this=0xcf12d89c, aTask=...) at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:1955
#18 0xc8b2f791 in mozilla::ipc::MessageChannel::MessageTask::Run() (this=0xafd4c500) at /home/emilio/src/moz/gecko-4/ipc/glue/MessageChannel.cpp:1986
#19 0xc84f90b8 in mozilla::SchedulerGroup::Runnable::Run() (this=0xafd260d0) at /home/emilio/src/moz/gecko-4/xpcom/threads/SchedulerGroup.cpp:295
#20 0xc850e422 in nsThread::ProcessNextEvent(bool, bool*) (this=0xcf1280c0, aMayWait=<optimized out>, aResult=0xcfe89b47) at /home/emilio/src/moz/gecko-4/xpcom/threads/nsThread.cpp:1225
#21 0xc8510390 in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0xcf1280c0, aMayWait=<optimized out>) at /home/emilio/src/moz/gecko-4/xpcom/threads/nsThreadUtils.cpp:486
#22 0xc8b31e6b in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0xcf109250, aDelegate=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/glue/MessagePump.cpp:110
#23 0xc8ab8517 in MessageLoop::RunInternal() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:315
#24 0xc8ab8485 in MessageLoop::Run() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:308
#25 0xc8ab8485 in MessageLoop::Run() (this=0xcfe89d50) at /home/emilio/src/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:290
#26 0xcb12d735 in nsBaseAppShell::Run() (this=0xcf12bb80) at /home/emilio/src/moz/gecko-4/widget/nsBaseAppShell.cpp:137

But then we never get a resize event at all, which we'd handle properly and update the dimensions for here...

So this is a regression from bug 1528052.

Regressed by: 1528052

So, multiple things going wrong here:

  • The size is initially 0x0. Why? Shouldn't we be able to start off with the correct viewport size? That's a question for James.
  • The resize event is suppressed. That's bug 1528052.

Hiro, why does bug 1528052 suppress the resize event? Shouldn't it just check whether the size has actually changed? It looks to me like otherwise websites have no way to handle situations like this, or when the phone is resized while the tab hasn't been painted yet.

A simple test-case here: https://crisal.io/tmp/gecko-mobile-resize.html

That is just a link to a page that writes the initial viewport size, and has a link to itself.

If you open it on Fenix, on the foreground, it has the right viewport size. If you tap the link and open it in a new tab it shows 0x0.

Seems like other websites are going to be broken as a result, so I think the right thing is:

  • Fix Geckoview so that we start off with the right viewport.
  • Fix Gecko so that we fire a resize event every time the viewport size actually changes.

(2) on its own may regress that compat bug, but along with (1) it shouldn't.

Hiro, James, does that seem reasonable?

Flags: needinfo?(hikezoe.birchill)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

So, multiple things going wrong here:

  • The size is initially 0x0. Why? Shouldn't we be able to start off with the correct viewport size? That's a question for James.
  • The resize event is suppressed. That's bug 1528052.

Hiro, why does bug 1528052 suppress the resize event? Shouldn't it just check whether the size has actually changed? It looks to me like otherwise websites have no way to handle situations like this, or when the phone is resized while the tab hasn't been painted yet.

The intention in bug 1528052 was to suppress a resize event that is caused by the very first reflow. But it seems that it suppresses a bunch of resize events unfortunately?

  • Fix Geckoview so that we start off with the right viewport.
  • Fix Gecko so that we fire a resize event every time the viewport size actually changes.

IIUC, we start with NS_UNCONSTRAINEDSIZE and we should ignore the change from the NS_UNCONSTRAINEDSIZE to a value.

  • The size is initially 0x0. Why? Shouldn't we be able to start off with the correct viewport size? That's a question for James.

Probably it's in a background tab? I am wondering why this issue doesn't happen on Fennec.

Anyway, I want the "open link in background tab" menu on GeckoView Example.

Flags: needinfo?(hikezoe.birchill)

Clearing NI, since Emilio got stuff working in r-b.

Flags: needinfo?(snorp)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #13)

Emilio got stuff working in r-b.

Emilio, any updates on this image scaling bug in Fenix and R-B? Are you actively investigating? This bug is a priority for the Fenix team.

Flags: needinfo?(emilio)

(This actually is a regression, I don't know why people keep removing the keyword)

I provided a diagnostic, a test-case and a regressing bug. I'd be happy to look into fixing this, but verifying the fix works is going to be a pain if I have to do this every time I rebuild Geckoview.

I can try to look into adding an "open in the background" kind of thing that does it in GVE or such. Though that's probably trivial, it'd take me at least quite some time to go through the android and Geckoview documentation / code to figure out what's going on, and I also have other stuff to do.

Is there any chance someone from the GeckoView team could do that / mentor me into doing that first?

Otherwise unless this gets extremely high in my priority list I don't think I'll have cycles to spare on it.

Flags: needinfo?(emilio) → needinfo?(cpeterson)
Keywords: regression

Here's a try run which fixes this and I think should fix this properly: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a09b22b3b0b3c2efdca072f4cc4acb6d867f4b9

Assignee: nobody → emilio
Depends on: 1577258

This bug is blocked waiting for Emilio's patches in bug 1577258. Since he wants to land those patches as incrementally (since this code is non-trivial), I assume we might not have all the patches landed in GV 70 Nightly before Nightly rides to Beta on September 3.

Flags: needinfo?(cpeterson)

Adding this bug to GV's September sprint.

status-firefox70: affected → fix-optional
Whiteboard: [geckoview:fenix:m7] → [geckoview:m1909]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #16)

Here's a try run which fixes this and I think should fix this properly: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a09b22b3b0b3c2efdca072f4cc4acb6d867f4b9

Emilio, do you have any updates on this Android image resizing bug? Your fixes for related ResizeReflowIgnoreOverride bug 1577258 landed last week.

status-firefox70: fix-optional → wontfix
status-firefox71: --- → affected
Flags: needinfo?(emilio)

I'm in TPAC this week, so no.

Depends on: 1583534
Flags: needinfo?(emilio)

This fixes the bug by always firing resize events when the layout viewport
changes.

The key here is that for mobile pages that reload on resize we were getting a
redundant resize event before, but now we no longer do.

In particular, consider the test-case:

<!doctype html>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<a href="" target="_blank">Open me in a separate tab</a>
<pre id="log"></pre>
<script>
// This shouldn't be needed, but otherwise Fenix doesn't show the tooltip on
// longpress...
document.querySelector("a").href = location.href;

function logSize() {
log.innerText += window.innerWidth + "x" + window.innerHeight + "\n";
}
logSize();
onresize = logSize;
</script>

(Hosted at https://crisal.io/tmp/gecko-mobile-resize.html for convenience)

Right now on trunk, when you click the link from GVE or Fenix, we're only
getting an initial size of 0x0 (which is not great, btw), and only after first
paint we get the real device size, but content doesn't get a resize event.

This is obviously wrong, every time the layout viewport changes we should fire
resize events.

Pages that get opened in new tabs and get refreshed when resized may get an
extra reload with this approach, but this seems not avoidable unless widget sets
the viewport size right in advance (which from discussion with snorp and agi
doesn't seem possible in the general case).

What used to happen is that we were triggering a redundant resize reflow from
the initial paint which didn't update the layout viewport (because the content
viewer and co had the right viewport from the previous navigation).

Now that we optimize those away, I think our behavior should be correct.

One thing we could try for the initial size is just using the size of the most recently created window. This has a high chance of being correct and is easy to do. I'll file a followup.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ab74e2147c5c
Remove MVMContext::ResizeEventFlag and related code. r=botond,hiro

https://hg.mozilla.org/mozilla-central/rev/ab74e2147c5c

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox71: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: