Bug 144672 - [EFL] Remove redudant condition in ViewClientEfl::didChangeViewportAttributes
Summary: [EFL] Remove redudant condition in ViewClientEfl::didChangeViewportAttributes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-05 23:39 PDT by Ryuan Choi
Modified: 2015-05-06 01:18 PDT (History)
2 users (show)

See Also:

Attachments
Patch (2.08 KB, patch)
2015-05-05 23:42 PDT, Ryuan Choi 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2015-05-05 23:39:32 PDT 
ViewClientEfl::didChangeViewportAttributes() is always called when FixedLayout is enabled.
Comment 1 Ryuan Choi 2015-05-05 23:42:21 PDT 
Created attachment 252455 [details]
Patch
Comment 2 Gyuyoung Kim 2015-05-05 23:49:26 PDT 
Comment on attachment 252455 [details]
Patch

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

> Source/WebKit2/UIProcess/efl/ViewClientEfl.cpp:-134
> -    ewkView->scheduleUpdateDisplay();

Why is this call removed ?

> Source/WebKit2/UIProcess/efl/ViewClientEfl.cpp:129
> +    ASSERT(WKPageUseFixedLayout(ewkView->wkPage()));

I wonder why we need to add assert(). Should we generate a crash though this function is always called when fixed layout is disabled ?
Comment 3 Ryuan Choi 2015-05-06 00:18:34 PDT 
Comment on attachment 252455 [details]
Patch

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

>> Source/WebKit2/UIProcess/efl/ViewClientEfl.cpp:-134
>> -    ewkView->scheduleUpdateDisplay();
> 
> Why is this call removed ?

It has never been called because this method is always called when fixed layout is enabled.

>> Source/WebKit2/UIProcess/efl/ViewClientEfl.cpp:129
>> +    ASSERT(WKPageUseFixedLayout(ewkView->wkPage()));
> 
> I wonder why we need to add assert(). Should we generate a crash though this function is always called when fixed layout is disabled ?

In the current logic, it will not be happened that this function is called without fixed layout.

I added assert() to catch the some bad changes earily in the debug build and notify that this method is only called when fixed layout is enabled.
Comment 4 Gyuyoung Kim 2015-05-06 00:22:14 PDT 
Comment on attachment 252455 [details]
Patch

ok, r=me.
Comment 5 WebKit Commit Bot 2015-05-06 01:18:52 PDT 
Comment on attachment 252455 [details]
Patch

Clearing flags on attachment: 252455

Committed r183864: <http://trac.webkit.org/changeset/183864>
Comment 6 WebKit Commit Bot 2015-05-06 01:18:56 PDT 
All reviewed patches have been landed.  Closing bug.