Bug 186585 – [Mail] Use the Mail Viewer width as the base for resolving horizontal viewport units

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

Instead of the stretched viewport size

[alan]

<rdar://problem/30685325>

[alan]

Created attachment 342624 [details]
Patch

[Tim Horton]

Comment on attachment 342624 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342624&action=review

> Source/WebCore/page/FrameView.cpp:4378
> +        // Only override the width of the viewport.

This is an unhelpful what comment instead of a why comment.

> Source/WebCore/page/FrameView.h:797
> +        operator IntSize() const
> +        {
> +            ASSERT(width);
> +            ASSERT(height);
> +            return { *width, *height };
> +        }

This seems ever so slightly dangerous. Maybe we should just always make callers unwrap?

[alan]

Created attachment 342663 [details]
Patch

[EWS Watchlist]

Comment on attachment 342663 [details]
Patch

Attachment 342663 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/8164301

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 342665 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[alan]

(In reply to Build Bot from comment #5)
> Comment on attachment 342663 [details]
> Patch
> 
> Attachment 342663 [details] did not pass mac-ews (mac):
> Output: http://webkit-queues.webkit.org/results/8164301
> 
> Number of test failures exceeded the failure limit.
something makes autosizing sticky.

[EWS Watchlist]

Comment on attachment 342663 [details]
Patch

Attachment 342663 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/8164327

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 342669 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6

[alan]

(In reply to zalan from comment #7)
> (In reply to Build Bot from comment #5)
> > Comment on attachment 342663 [details]
> > Patch
> > 
> > Attachment 342663 [details] did not pass mac-ews (mac):
> > Output: http://webkit-queues.webkit.org/results/8164301
> > 
> > Number of test failures exceeded the failure limit.
> something makes autosizing sticky.
Oh it's WK1 only. -and I am actually able to repro it locally.

[alan]

(In reply to zalan from comment #10)
> (In reply to zalan from comment #7)
> > (In reply to Build Bot from comment #5)
> > > Comment on attachment 342663 [details]
> > > Patch
> > > 
> > > Attachment 342663 [details] did not pass mac-ews (mac):
> > > Output: http://webkit-queues.webkit.org/results/8164301
> > > 
> > > Number of test failures exceeded the failure limit.
> > something makes autosizing sticky.
> Oh it's WK1 only. -and I am actually able to repro it locally.
and these tests start failing the same way even without the patch (ofc) when only the test case is added.

[alan]

(In reply to zalan from comment #11)
> (In reply to zalan from comment #10)
> > (In reply to zalan from comment #7)
> > > (In reply to Build Bot from comment #5)
> > > > Comment on attachment 342663 [details]
> > > > Patch
> > > > 
> > > > Attachment 342663 [details] did not pass mac-ews (mac):
> > > > Output: http://webkit-queues.webkit.org/results/8164301
> > > > 
> > > > Number of test failures exceeded the failure limit.
> > > something makes autosizing sticky.
> > Oh it's WK1 only. -and I am actually able to repro it locally.
> and these tests start failing the same way even without the patch (ofc) when
> only the test case is added.
Forcing a "resize(800, 600)" while turning the autosizing off fixes it. :(

[alan]

Created attachment 342680 [details]
Patch

[alan]

Created attachment 342689 [details]
Patch

[Tim Horton]

Comment on attachment 342689 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342689&action=review

> Source/WebCore/page/FrameView.h:216
> +

No

[alan]

Created attachment 342694 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 342694 [details]
Patch

Clearing flags on attachment: 342694

Committed r232823: <https://trac.webkit.org/changeset/232823>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41110634>

[Matt Lewis]

This revision has caused both a test to start timing out consistently on WK1 and a consistent failure of an api test:

the test:
fast/dynamic/paused-event-dispatch.html
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fdynamic%2Fpaused-event-dispatch.html
diff:

--- /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/fast/dynamic/paused-event-dispatch-expected.txt
+++ /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/fast/dynamic/paused-event-dispatch-actual.txt
@@ -1,5 +1,33 @@
-Test for http://bugs.webkit.org/show_bug.cgi?id=16490 ASSERT in ~FrameView while viewing/reloading WICD test case.
-
-
-PASS
-
+FAIL: Timed out waiting for notifyDone to be called
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 54x18
+          text run at (0,0) width 54: "Test for "
+        RenderInline {I} at (0,0) size 749x36
+          RenderInline {A} at (0,0) size 305x18 [color=#0000EE]
+            RenderText {#text} at (53,0) size 305x18
+              text run at (53,0) width 305: "http://bugs.webkit.org/show_bug.cgi?id=16490"
+          RenderText {#text} at (357,0) size 749x36
+            text run at (357,0) width 5: " "
+            text run at (361,0) width 388: "ASSERT in ~FrameView while viewing/reloading WICD test"
+            text run at (0,18) width 29: "case"
+        RenderText {#text} at (28,18) size 5x18
+          text run at (28,18) width 5: "."
+      RenderBlock {DIV} at (0,52) size 784x154
+        RenderIFrame {IFRAME} at (0,0) size 788x154 [border: (2px inset #000000)]
+          layer at (0,0) size 784x150
+            RenderView at (0,0) size 784x150
+          layer at (0,0) size 784x150
+            RenderBlock {HTML} at (0,0) size 784x150
+              RenderBody {BODY} at (8,8) size 768x134
+        RenderText {#text} at (0,0) size 0x0
+      RenderBlock {DIV} at (0,306) size 784x18
+        RenderText {#text} at (0,0) size 35x18
+          text run at (0,0) width 35: "FAIL"
+layer at (8,214) size 100x100 clip at (8,214) size 85x100 scrollHeight 200
+  RenderBlock {DIV} at (0,206) size 100x100
+    RenderBlock {DIV} at (0,0) size 85x200



build:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/5985


API failure:

TestWebKitAPI.WebKit.AutoLayoutIntegration
        
        /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/AutoLayoutIntegration.mm:99
        Value of: intrinsicContentSize.height
          Actual: 101
        Expected: _expectedIntrinsicContentSize.height
        Which is: 150

https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/5985/steps/run-api-tests/logs/stdio

[Matt Lewis]

Reverted r232823 for reason:

This caused both test timeouts and api failures.

Committed r232859: <https://trac.webkit.org/changeset/232859>

[alan]

(In reply to Matt Lewis from comment #20)
> This revision has caused both a test to start timing out consistently on WK1
> and a consistent failure of an api test:
> 
> the test:
> fast/dynamic/paused-event-dispatch.html
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=fast%2Fdynamic%2Fpaused-event-dispatch.html
> diff:
> 
> ---
> /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/
> fast/dynamic/paused-event-dispatch-expected.txt
> +++
> /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/
> fast/dynamic/paused-event-dispatch-actual.txt
> @@ -1,5 +1,33 @@
> -Test for http://bugs.webkit.org/show_bug.cgi?id=16490 ASSERT in ~FrameView
> while viewing/reloading WICD test case.
> -
> -
> -PASS
> -
> +FAIL: Timed out waiting for notifyDone to be called
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x600
> +  RenderBlock {HTML} at (0,0) size 800x600
> +    RenderBody {BODY} at (8,8) size 784x584
> +      RenderBlock {P} at (0,0) size 784x36
> +        RenderText {#text} at (0,0) size 54x18
> +          text run at (0,0) width 54: "Test for "
> +        RenderInline {I} at (0,0) size 749x36
> +          RenderInline {A} at (0,0) size 305x18 [color=#0000EE]
> +            RenderText {#text} at (53,0) size 305x18
> +              text run at (53,0) width 305:
> "http://bugs.webkit.org/show_bug.cgi?id=16490"
> +          RenderText {#text} at (357,0) size 749x36
> +            text run at (357,0) width 5: " "
> +            text run at (361,0) width 388: "ASSERT in ~FrameView while
> viewing/reloading WICD test"
> +            text run at (0,18) width 29: "case"
> +        RenderText {#text} at (28,18) size 5x18
> +          text run at (28,18) width 5: "."
> +      RenderBlock {DIV} at (0,52) size 784x154
> +        RenderIFrame {IFRAME} at (0,0) size 788x154 [border: (2px inset
> #000000)]
> +          layer at (0,0) size 784x150
> +            RenderView at (0,0) size 784x150
> +          layer at (0,0) size 784x150
> +            RenderBlock {HTML} at (0,0) size 784x150
> +              RenderBody {BODY} at (8,8) size 768x134
> +        RenderText {#text} at (0,0) size 0x0
> +      RenderBlock {DIV} at (0,306) size 784x18
> +        RenderText {#text} at (0,0) size 35x18
> +          text run at (0,0) width 35: "FAIL"
> +layer at (8,214) size 100x100 clip at (8,214) size 85x100 scrollHeight 200
> +  RenderBlock {DIV} at (0,206) size 100x100
> +    RenderBlock {DIV} at (0,0) size 85x200
> 
> 
> 
> build:
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/5985
> 
> 
> API failure:
> 
> TestWebKitAPI.WebKit.AutoLayoutIntegration
>         
>        
> /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/
> WebKitCocoa/AutoLayoutIntegration.mm:99
>         Value of: intrinsicContentSize.height
>           Actual: 101
>         Expected: _expectedIntrinsicContentSize.height
>         Which is: 150
> 
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/5985/steps/run-api-
> tests/logs/stdio
The API expected test needs rebaselining and the timeout look suspicious since the testcase does not even turn on this (autosizing) feature (does not exercise this changeset) Will look into it.

[alan]

Created attachment 343446 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 343446 [details]
Patch

Clearing flags on attachment: 343446

Committed r233132: <https://trac.webkit.org/changeset/233132>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[alan]

(In reply to Matt Lewis from comment #20)
> This revision has caused both a test to start timing out consistently on WK1
> and a consistent failure of an api test:
> 
> the test:
> fast/dynamic/paused-event-dispatch.html
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=fast%2Fdynamic%2Fpaused-event-dispatch.html
> diff:
> 
> ---
> /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/
> fast/dynamic/paused-event-dispatch-expected.txt
> +++
> /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/
> fast/dynamic/paused-event-dispatch-actual.txt
> @@ -1,5 +1,33 @@
> -Test for http://bugs.webkit.org/show_bug.cgi?id=16490 ASSERT in ~FrameView
> while viewing/reloading WICD test case.
> -
> -
> -PASS
> -
> +FAIL: Timed out waiting for notifyDone to be called
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x600
> +  RenderBlock {HTML} at (0,0) size 800x600
> +    RenderBody {BODY} at (8,8) size 784x584
> +      RenderBlock {P} at (0,0) size 784x36
> +        RenderText {#text} at (0,0) size 54x18
> +          text run at (0,0) width 54: "Test for "
> +        RenderInline {I} at (0,0) size 749x36
> +          RenderInline {A} at (0,0) size 305x18 [color=#0000EE]
> +            RenderText {#text} at (53,0) size 305x18
> +              text run at (53,0) width 305:
> "http://bugs.webkit.org/show_bug.cgi?id=16490"
> +          RenderText {#text} at (357,0) size 749x36
> +            text run at (357,0) width 5: " "
> +            text run at (361,0) width 388: "ASSERT in ~FrameView while
> viewing/reloading WICD test"
> +            text run at (0,18) width 29: "case"
> +        RenderText {#text} at (28,18) size 5x18
> +          text run at (28,18) width 5: "."
> +      RenderBlock {DIV} at (0,52) size 784x154
> +        RenderIFrame {IFRAME} at (0,0) size 788x154 [border: (2px inset
> #000000)]
> +          layer at (0,0) size 784x150
> +            RenderView at (0,0) size 784x150
> +          layer at (0,0) size 784x150
> +            RenderBlock {HTML} at (0,0) size 784x150
> +              RenderBody {BODY} at (8,8) size 768x134
> +        RenderText {#text} at (0,0) size 0x0
> +      RenderBlock {DIV} at (0,306) size 784x18
> +        RenderText {#text} at (0,0) size 35x18
> +          text run at (0,0) width 35: "FAIL"
> +layer at (8,214) size 100x100 clip at (8,214) size 85x100 scrollHeight 200
> +  RenderBlock {DIV} at (0,206) size 100x100
> +    RenderBlock {DIV} at (0,0) size 85x200
> 
> 
> 
> build:
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/5985
> 
> 
> API failure:
> 
> TestWebKitAPI.WebKit.AutoLayoutIntegration
>         
>        
> /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/
> WebKitCocoa/AutoLayoutIntegration.mm:99
>         Value of: intrinsicContentSize.height
>           Actual: 101
>         Expected: _expectedIntrinsicContentSize.height
>         Which is: 150
> 
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/5985/steps/run-api-
> tests/logs/stdio
Fixed both the timeout and the API test failure.