Here's the analysis:

- tab_drag_controller remembers if the original window was maximized or in fullscreen
- at the end of the dragging, it checks this flag and recovers to the original status (maximized or fullscreen) as https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/tab_drag_controller.cc?sq=package:chromium&g=0&l=1730
- this is guarded by IsSnapped() -- so if it's snapped, then it's not maximized https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/tab_drag_controller.cc?sq=package:chromium&g=0&l=1723
- there's an assumption that this 'IsSnapped' state is set synchronously;
   - split_view_controller watches 'is-tab-dragging' state, and set the snapped window state upon its ending (https://cs.chromium.org/chromium/src/ash/wm/splitview/split_view_controller.cc?q=split_view_controller&sq=package:chromium&g=0&l=171)
   - it is cleared in TabDragController::ClearTabDraggingInfo() as https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/tab_drag_controller.cc?sq=package:chromium&g=0&l=2113
   - the code assumes those updates happen synchronously -- TabDragController::EndDragImpl() invokes ClearTabDraggingInfo() and then invokes CompleteDrag() immediately, assuming SplitViewController::OnWindowPropertyChanged is invoked in the middle.
   - on mash, this is not the case, SplitViewController::OnWindowPropertyChanged will be called asynchronously and therefore IsSnapped will return false.

mukai@, thanks for the analysis.

For the fix in mash, maybe we can let SplitViewController fully decide where to place the window after the drag ends since SplitViewController has the full knowledge? 

Yeah I think the cleaner solution would be letting SplitViewController make the decision, though we shouldn't break desktop-mode behaviors. For example, maximized window's tab can be dragged in desktop mode, which should resume to be maximized after the dragging. Maybe those status control should also be done in Ash rather than TabDragController for ChromeOS?

Yes I agree those status control can be done in Ash in Chrome OS. We probably still need to preserve the original logic in TabDragController for desktop behaviors though.

CL is https://chromium-review.googlesource.com/c/chromium/src/+/1214175, but for the record, I've changed the approach because of fullscreen state;

When the tab dragging has ended without snapped state, someone needs to recover the window to the original state (maximized or fullscreen), and this fullscreen has to be the immersive-fullscreen which is activated through chrome::ToggleFullscreenMode(). That means, this handling of recovering window state needs to be under chrome/browser/ui.

Therefore, handling those things within SplitViewController isn't working well unfortunately. Instead of doing this, an idea is to send the WindowStateType property earlier than actual processing.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c913f997537e0aa18efda5ce22044f97574fe98a

commit c913f997537e0aa18efda5ce22044f97574fe98a
Author: Jun Mukai <mukai@chromium.org>
Date: Thu Sep 13 18:29:48 2018

Sends WindowStateType property earlier on snapped tablet-mode

When the window is snapped to left or right at the end of
tab-dragging in tablet-mode, the snapped state is needed by
TabDragController before the actual process of snap window
happens.

Because of this, SplitViewController sends the window state
type first, and then wait for the end of tab-dragging to
process the snapping of window.

BUG= 880635 
TEST=the new expectation covers

Change-Id: I7e37f2a32c4bcf66fb3106c5ab8fcce6be98a5fd
Reviewed-on: https://chromium-review.googlesource.com/1214175
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591079}
[modify] https://crrev.com/c913f997537e0aa18efda5ce22044f97574fe98a/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/c913f997537e0aa18efda5ce22044f97574fe98a/ash/public/cpp/window_properties.h
[modify] https://crrev.com/c913f997537e0aa18efda5ce22044f97574fe98a/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/c913f997537e0aa18efda5ce22044f97574fe98a/ash/wm/splitview/split_view_controller.cc
[modify] https://crrev.com/c913f997537e0aa18efda5ce22044f97574fe98a/ash/wm/splitview/split_view_controller_unittest.cc
[modify] https://crrev.com/c913f997537e0aa18efda5ce22044f97574fe98a/chrome/browser/ui/views/tabs/tab_drag_controller.cc

This seems happening again. Possibly a transient error due to estade's refactoring of window frame?

My first guess would be that it's not related to my refactoring. I investigated a little, and I think it's due to the line that was added in TabDragController::ClearTabDraggingInfo():

  dragged_window->ClearProperty(ash::kTabDroppedWindowStateTypeKey);

ClearTabDraggingInfo() is called just before CompleteDrag(), in EndDragImpl(). When I comment out that line, which wasn't present in the initial upload but was added after this comment[1], it works as expected.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1214175/4/ash/wm/splitview/split_view_controller.cc#598

Thanks, you're right. Maybe this is just my failure, it wasn't fixed by my CL at all :-(
Created a CL to change the timing of ClearTagDraggingInfo().

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6b5fd715285b031a90fc1f56bdcb1c3fa3d2fb0

commit c6b5fd715285b031a90fc1f56bdcb1c3fa3d2fb0
Author: Jun Mukai <mukai@chromium.org>
Date: Tue Oct 02 23:27:55 2018

fix TabDragging snap on tablet mode with Mash

The reported bug (tab-dragging to snap to left/right on tablet
mode with Mash) happens due to the timing of ClearTabDraggingInfo(),
it clears ash::kTabDroppedWindowStateTypeKey but this property
is used on IsSnapped() function.

To have this property, this CL moves invocation of
ClearTabDraggingInfo() slightly later in EndDragImpl, so that
CompleteDrag() can use the values.

BUG= 880635 
TEST=existing test passes, assuming this break nothing

Change-Id: I8ba34c793dade6462fb1ee9470851cf8f066c4ad
Reviewed-on: https://chromium-review.googlesource.com/c/1256006
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596040}
[modify] https://crrev.com/c6b5fd715285b031a90fc1f56bdcb1c3fa3d2fb0/chrome/browser/ui/views/tabs/tab_drag_controller.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66f2b045c8fbf0248069127963dfb203d359ebaa

commit 66f2b045c8fbf0248069127963dfb203d359ebaa
Author: Jun Mukai <mukai@chromium.org>
Date: Thu Oct 04 18:49:13 2018

Revert "fix TabDragging snap on tablet mode with Mash"

This reverts commit c6b5fd715285b031a90fc1f56bdcb1c3fa3d2fb0.

Reason for revert: breaks tablet-mode tab-dragging

Original change's description:
> fix TabDragging snap on tablet mode with Mash
> 
> The reported bug (tab-dragging to snap to left/right on tablet
> mode with Mash) happens due to the timing of ClearTabDraggingInfo(),
> it clears ash::kTabDroppedWindowStateTypeKey but this property
> is used on IsSnapped() function.
> 
> To have this property, this CL moves invocation of
> ClearTabDraggingInfo() slightly later in EndDragImpl, so that
> CompleteDrag() can use the values.
> 
> BUG= 880635 
> TEST=existing test passes, assuming this break nothing
> 
> Change-Id: I8ba34c793dade6462fb1ee9470851cf8f066c4ad
> Reviewed-on: https://chromium-review.googlesource.com/c/1256006
> Commit-Queue: Jun Mukai <mukai@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596040}

TBR=mukai@chromium.org,sky@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  880635 ,  892221 
Change-Id: Ib9dc6ee2f2e9c9572c770d70435b7d9d088f8dd9
Reviewed-on: https://chromium-review.googlesource.com/c/1261932
Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596783}
[modify] https://crrev.com/66f2b045c8fbf0248069127963dfb203d359ebaa/chrome/browser/ui/views/tabs/tab_drag_controller.cc

reverted since this CL breaks some behavior (see  issue 892221  for the details).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c20cccbebb87def5758d150a6caa34b3bd415565

commit c20cccbebb87def5758d150a6caa34b3bd415565
Author: Jun Mukai <mukai@chromium.org>
Date: Mon Oct 08 18:20:23 2018

Fix of tab-dragging in tablet-mode with Mash

To process Left/Right snapping properly, CompleteDrag() in
TabDragController needs some properties, thus r596783 moves
ClearTabDraggingInfo() after CompleteDrag(). However
 https://crbug.com/892221  revealed that another propertly
kIsDraggingTabsKey needs to be cleared beforehand, otherwise
some bounds calculation goes wrong.

This CL moves ClearTabDraggingInfo() as my previous CL does,
but clears kIsDraggingTabsKey earlier so  issue 892221  won't
be affected.

BUG= 880635 ,  892221 
TEST=manually

Change-Id: Ib7edaab5e968e006bc919b027c138ad9f3d5827d
Reviewed-on: https://chromium-review.googlesource.com/c/1266495
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597616}
[modify] https://crrev.com/c20cccbebb87def5758d150a6caa34b3bd415565/chrome/browser/ui/views/tabs/tab_drag_controller.cc
[modify] https://crrev.com/c20cccbebb87def5758d150a6caa34b3bd415565/chrome/browser/ui/views/tabs/tab_drag_controller.h