Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2248793003:
[Android] Remove some dead code in CompositorViewHolder (second try) (Closed)
Description[Android] Remove some dead code in CompositorViewHolder
After https://codereview.chromium.org/2241643003, Tab#getView() will
only return the wrapper framelyout that wraps infobars, content view or
native page view. This work simplifies the logic in
CompositorViewHolder.
BUG=635871
Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af
Committed: https://crrev.com/760c7b9046d15e6747a3c230225ff238bc2538f7
Cr-Original-Commit-Position: refs/heads/master@{#412133}
Cr-Commit-Position: refs/heads/master@{#412673}
Patch Set 1 #Patch Set 2 : revert of revert. Found the reason of crash #
Total comments: 1
Patch Set 3 : grammar #
Messages
Total messages: 23 (12 generated)
ianwen@chromium.org changed reviewers: + dfalcantara@chromium.org
PTAL. Note the margins of native pages are now handled in TabContentViewParent, and CompositorViewHolder should not care about margin anymore. Also since there is only one type of view Tab#getView() returns, I directly made it to have the correct type.
lgtm
The CQ bit was checked by ianwen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 ========== to ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Cr-Commit-Position: refs/heads/master@{#412133} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Cr-Commit-Position: refs/heads/master@{#412133}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2250113003/ by mvanouwerkerk@chromium.org. The reason for reverting is: See https://bugs.chromium.org/p/chromium/issues/detail?id=638535 This crashes due to: 08-17 10:49:47.072 15523 15523 W System.err: java.lang.ClassCastException: android.widget.FrameLayout$LayoutParams cannot be cast to android.support.design.widget.CoordinatorLayout$LayoutParams 08-17 10:49:47.073 15523 15523 W System.err: at org.chromium.chrome.browser.compositor.CompositorViewHolder.updateContentOverlayVisibility(CompositorViewHolder.java:808).
Message was sent while issue was closed.
Hi Dan, I found the reason of the crash. Would you mind taking another look? Sorry for the hassle. :(
Message was sent while issue was closed.
Description was changed from ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Cr-Commit-Position: refs/heads/master@{#412133} ========== to ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Cr-Commit-Position: refs/heads/master@{#412133} ==========
The CQ bit was checked by ianwen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm https://chromiumcodereview.appspot.com/2248793003/diff/20001/chrome/android/j... File chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java (right): https://chromiumcodereview.appspot.com/2248793003/diff/20001/chrome/android/j... chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java:793: // measure and layout it. This way we could show the animation in the stack view. lay it out perform layout
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ianwen@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dfalcantara@chromium.org Link to the patchset: https://codereview.chromium.org/2248793003/#ps40001 (title: "grammar")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Cr-Commit-Position: refs/heads/master@{#412133} ========== to ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Cr-Commit-Position: refs/heads/master@{#412133} ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Cr-Commit-Position: refs/heads/master@{#412133} ========== to ========== [Android] Remove some dead code in CompositorViewHolder After https://codereview.chromium.org/2241643003, Tab#getView() will only return the wrapper framelyout that wraps infobars, content view or native page view. This work simplifies the logic in CompositorViewHolder. BUG=635871 Committed: https://crrev.com/01dca5c74d54e47c725ae94d7714884bf51f21af Committed: https://crrev.com/760c7b9046d15e6747a3c230225ff238bc2538f7 Cr-Original-Commit-Position: refs/heads/master@{#412133} Cr-Commit-Position: refs/heads/master@{#412673} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/760c7b9046d15e6747a3c230225ff238bc2538f7 Cr-Commit-Position: refs/heads/master@{#412673} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
