Bug 114947 – [EFL] Prevent to apply a large scale factor on launching the MiniBrowser.

• Component: WebKit EFL
• Status: RESOLVED WONTFIX

[JungJik Lee]

when we launch the MiniBrowser, sometimes the browser is stopped for a while. It is owing to a large scale factor. the ewk_view size is 128x5(w,h) at initial time and to fit to web page to view size, the browser applies a large scale factor in updateMinimumScaleToFit function up to scale 70.0f. On the other hand WebProcess is parsing the page, so when the scale factor is applied to backing store,  the actual page size becomes 800x600.  this makes the TBS insanely big (800 width * 70.0f).
the browser is stopped for drawing all tiles with that size. So what makes the view size grow? because we use EVAS_HINT_EXPAND in ewk_view. this flag makes the evas increase the size as time goes by. we should limit the hint size to fix size evas.
evas_object_size_hint_min_set function give a limitation to HINT_EXPAND.

[JungJik Lee]

Created attachment 199015 [details]
Patch

[JungJik Lee]

Created attachment 199215 [details]
Patch

[Gyuyoung Kim]

CC'ing graphics folks.

[Gyuyoung Kim]

Comment on attachment 199215 [details]
Patch

It looks this patch makes sense.

[Ryuan Choi]

Comment on attachment 199215 [details]
Patch

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

> Tools/MiniBrowser/efl/main.c:1566
> +    evas_object_size_hint_min_set(window->ewk_view, width ? width : window_width, height ? height : window_height);

Could you try after moved evas_object_resize (at 1574 line) here ?

This code added wrong behavior which does not reduce webview size when resized window smaller.

[JungJik Lee]

(In reply to comment #5)
> (From update of attachment 199215 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=199215&action=review
> 
> > Tools/MiniBrowser/efl/main.c:1566
> > +    evas_object_size_hint_min_set(window->ewk_view, width ? width : window_width, height ? height : window_height);
> 
> Could you try after moved evas_object_resize (at 1574 line) here ?
> 
> This code added wrong behavior which does not reduce webview size when resized window smaller.

Yes, I know that. so I asked not to commit this patch to gyuyoung until I check that you mentioned. I'll check it again. thanks your comment.

[JungJik Lee]

Created attachment 201013 [details]
Patch

[JungJik Lee]

I explain more about this issue.
The issue is that when we launch the MiniBrowser, the MiniBrowser is paused for 1 ~ 3 seconds often. It's a timing issue. this occurs only in EFL.

The cause is that we use elm_box for layout and the layouting is not finished at one go. Due to the other child boxes _els_box_layout(EFL function) is called three times for the layout calculation. As a result, the ewk_view also is resized like this.

EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=127, height=0)
EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=250, height=25)
EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=800, height=564)

And the actual problem is when ewk_view size is 127,0, UIProcess calls PageViewportController::updateMinimumScaleToFit and send the result scale factor (70.5f) to WebProcess. When WebProcess receives the result, TBS has a normal  viewport size layer. TBS creates tiles as large as multiplying by scale 800(width)x70.5(scale) and draws the tiles. That makes pausing.

I checked ryuan.choi's comment. Even we change the position of evas_object_resize, it is possible to be remained.

[Gyuyoung Kim]

Comment on attachment 201013 [details]
Patch

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

> Tools/ChangeLog:6
> +        Reviewed by Gyuyoung Kim.

Please remove my name. This patch needs to be reviewed again.

[Mikhail Pozdnyakov]

Comment on attachment 201013 [details]
Patch

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

looks like a work-around, might be ok thought

> Tools/ChangeLog:9
> +        resized from small size to default size gradually. This causes a large

what do you mean by "gradually"? web view size is set multiple times?

> Tools/ChangeLog:10
> +        scale factor in updateMinimumScaleToFit function. And the large scale

could you please point the class name also? PageViewportController::updateMinimumScaleToFit?

> Tools/ChangeLog:11
> +        factor is applied to TBS. As a result, the MiniBrowser is stopped for

TBS? Please avoid using acronyms in change log.

> Tools/MiniBrowser/efl/main.c:59
> +static Eina_Bool ignore_initial_layout = EINA_FALSE;

can be static inside the function.

> Tools/MiniBrowser/efl/main.c:418
> +on_resize(void *user_data, Evas *e, Evas_Object *ewk_view, void *event_info)

variables that are not used can be omitted.

[Ryuan Choi]

(In reply to comment #8)
> I explain more about this issue.
> The issue is that when we launch the MiniBrowser, the MiniBrowser is paused for 1 ~ 3 seconds often. It's a timing issue. this occurs only in EFL.
> 
> The cause is that we use elm_box for layout and the layouting is not finished at one go. Due to the other child boxes _els_box_layout(EFL function) is called three times for the layout calculation. As a result, the ewk_view also is resized like this.
> 
> EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=127, height=0)
> EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=250, height=25)
> EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=800, height=564)
> 
> And the actual problem is when ewk_view size is 127,0, UIProcess calls PageViewportController::updateMinimumScaleToFit and send the result scale factor (70.5f) to WebProcess. When WebProcess receives the result, TBS has a normal  viewport size layer. TBS creates tiles as large as multiplying by scale 800(width)x70.5(scale) and draws the tiles. That makes pausing.
> 
> I checked ryuan.choi's comment. Even we change the position of evas_object_resize, it is possible to be remained.

Doesn't we need to handle exception instead of just adding workaround for minibrowser?
Anybody might mistake to resize ewk view to zero width or height.

What do you think about it?

[JungJik Lee]

Comment on attachment 201013 [details]
Patch

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

>> Tools/ChangeLog:6
>> +        Reviewed by Gyuyoung Kim.
> 
> Please remove my name. This patch needs to be reviewed again.

Okay. I will.

>> Tools/ChangeLog:9
>> +        resized from small size to default size gradually. This causes a large
> 
> what do you mean by "gradually"? web view size is set multiple times?

yes, multiple times.

>> Tools/ChangeLog:10
>> +        scale factor in updateMinimumScaleToFit function. And the large scale
> 
> could you please point the class name also? PageViewportController::updateMinimumScaleToFit?

okay.

>> Tools/ChangeLog:11
>> +        factor is applied to TBS. As a result, the MiniBrowser is stopped for
> 
> TBS? Please avoid using acronyms in change log.

It means TiledBackingStore. Okay,I'll write a full name.

>> Tools/MiniBrowser/efl/main.c:59
>> +static Eina_Bool ignore_initial_layout = EINA_FALSE;
> 
> can be static inside the function.

Okay. I'll move it.

>> Tools/MiniBrowser/efl/main.c:418
>> +on_resize(void *user_data, Evas *e, Evas_Object *ewk_view, void *event_info)
> 
> variables that are not used can be omitted.

if we remove the parameter name, compiler shows this log > error: parameter name omitted.

[JungJik Lee]

Created attachment 201031 [details]
Patch

[JungJik Lee]

(In reply to comment #10)
> (From update of attachment 201013 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=201013&action=review
> 
> looks like a work-around, might be ok thought
> 
Yes, it looks like. but I couldn't find the best way to avoid this situation.
Thanks your comments.

> > Tools/ChangeLog:9
> > +        resized from small size to default size gradually. This causes a large
> 
> what do you mean by "gradually"? web view size is set multiple times?
> 
> > Tools/ChangeLog:10
> > +        scale factor in updateMinimumScaleToFit function. And the large scale
> 
> could you please point the class name also? PageViewportController::updateMinimumScaleToFit?
> 
> > Tools/ChangeLog:11
> > +        factor is applied to TBS. As a result, the MiniBrowser is stopped for
> 
> TBS? Please avoid using acronyms in change log.
> 
> > Tools/MiniBrowser/efl/main.c:59
> > +static Eina_Bool ignore_initial_layout = EINA_FALSE;
> 
> can be static inside the function.
> 
> > Tools/MiniBrowser/efl/main.c:418
> > +on_resize(void *user_data, Evas *e, Evas_Object *ewk_view, void *event_info)
> 
> variables that are not used can be omitted.

[JungJik Lee]

(In reply to comment #11)
> (In reply to comment #8)
> > I explain more about this issue.
> > The issue is that when we launch the MiniBrowser, the MiniBrowser is paused for 1 ~ 3 seconds often. It's a timing issue. this occurs only in EFL.
> > 
> > The cause is that we use elm_box for layout and the layouting is not finished at one go. Due to the other child boxes _els_box_layout(EFL function) is called three times for the layout calculation. As a result, the ewk_view also is resized like this.
> > 
> > EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=127, height=0)
> > EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=250, height=25)
> > EwkView::handleEvasObjectResize (evasObject=0x9b9370, width=800, height=564)
> > 
> > And the actual problem is when ewk_view size is 127,0, UIProcess calls PageViewportController::updateMinimumScaleToFit and send the result scale factor (70.5f) to WebProcess. When WebProcess receives the result, TBS has a normal  viewport size layer. TBS creates tiles as large as multiplying by scale 800(width)x70.5(scale) and draws the tiles. That makes pausing.
> > 
> > I checked ryuan.choi's comment. Even we change the position of evas_object_resize, it is possible to be remained.
> 
> Doesn't we need to handle exception instead of just adding workaround for minibrowser?
> Anybody might mistake to resize ewk view to zero width or height.
> 
> What do you think about it?

I missed this comment. this issue is happened because of elm box layout structure. if we don't use the elm box layout,it wouldn't happen. so I think it's better to handle this exception in minibrowser than EwkView.cpp.

[JungJik Lee]

the root cause of this issue is the imparity contentsSize of WebProcess and UIProcess. However it is going to remove sync message type from WK2, so I think it is better to handle this issue in MiniBrowser.

EBPROCESS virtual void WebKit::WebChromeClient::contentsSizeChanged(WebCore::Frame*, const WebCore::IntSize&) const 127 8
WEBPROCESS virtual void WebKit::WebChromeClient::contentsSizeChanged(WebCore::Frame*, const WebCore::IntSize&) const 800 564
UIPROCESS void WebKit::PageViewportController::didChangeContentsSize(const WebCore::IntSize&) 127 8
-> at this time if we calculate the scale factor, UIProcess know 127,8 size,
But at this time WebProcess 800,564.
UIPROCESS void WebKit::PageViewportController::didChangeContentsSize(const WebCore::IntSize&) 800 564
-> at this time WebProcess accept the scale factor and create a big TiledBackingStore.

check out this patch, https://bugs.webkit.org/show_bug.cgi?id=116105

[Gyuyoung Kim]

Comment on attachment 201031 [details]
Patch

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

I also think this isn't correct way to fix this problem. However, I think this problem has blocked EFL development. So, I'd like to land this patch at the moment. Any other objection or comment ?

> Tools/MiniBrowser/efl/main.c:419
> +    static Eina_Bool ignore_initial_layout = EINA_FALSE;

Could you add a comment why this logic we need using "FIXME:" ?

[JungJik Lee]

Created attachment 201917 [details]
Patch

[Build Bot]

Comment on attachment 201917 [details]
Patch

Attachment 201917 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/483182

New failing tests:
fast/events/mouse-cursor-image-set.html

[Build Bot]

Created attachment 201929 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

[Chris Dumez]

(In reply to comment #17)
> (From update of attachment 201031 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=201031&action=review
> 
> I also think this isn't correct way to fix this problem. However, I think this problem has blocked EFL development. So, I'd like to land this patch at the moment. Any other objection or comment ?

Is this really blocking EFL development? MiniBrowser seems to behave acceptably for me and it does not affect my work in any way.

I agree with previous comments that this feels like a hack and we would better take time to find a proper fix instead of landing it.

If this really blocks EFL development, please elaborate.

[Gyuyoung Kim]

(In reply to comment #21)
> (In reply to comment #17)
> > (From update of attachment 201031 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=201031&action=review
> > 
> > I also think this isn't correct way to fix this problem. However, I think this problem has blocked EFL development. So, I'd like to land this patch at the moment. Any other objection or comment ?
> 
> Is this really blocking EFL development? MiniBrowser seems to behave acceptably for me and it does not affect my work in any way.
> 
> I agree with previous comments that this feels like a hack and we would better take time to find a proper fix instead of landing it.

Yes, I agree that this patch is not proper fix.
 
> If this really blocks EFL development, please elaborate.

If you run MiniBrowser on Ubuntu 13.04, it is locked up frequently. I also think this should be fixed with proper solution though, we couldn't find proper fix during a month. If you think we need to find proper fix further, I don't mind it. But, if there is no good fix at the moment, please consider this patch again.

[Gyuyoung Kim]

Comment on attachment 201917 [details]
Patch

Doesn't this problem happen anymore ? If so, please close this bug.

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.