Speculative fix for crash in blink::WebFrameWidgetBase::dragTargetDragEnterOrOver

Speculative fix for crash in blink::WebFrameWidgetBase::dragTargetDragEnterOrOver.

BUG=671504
Review-Url: https://codereview.chromium.org/2695043002
Cr-Commit-Position: refs/heads/master@{#451149}
Committed: https://chromium.googlesource.com/chromium/src/+/1b7963b7210d4d1a45049841cf83eb30274ba82d

[paulmeyer, 2017-02-14 15:52:25]

Description was changed from

==========
# Enter a description of the change.

Speculative fix for crash in
blink::WebFrameWidgetBase::dragTargetDragEnterOrOver.

BUG=671504
==========

to

==========
Speculative fix for crash in
blink::WebFrameWidgetBase::dragTargetDragEnterOrOver.

BUG=671504
==========

[paulmeyer, 2017-02-14 15:53:17]

paulmeyer@chromium.org changed reviewers:
+ pdr@chromium.org

[pdr., 2017-02-15 18:11:35]

On 2017/02/14 at 15:53:17, paulmeyer wrote:
> 

I think this is the bug, but we might be able to reason about this crash and
make a real fix. It looks like we're somehow getting into the dragTargetDragOver
state without first going through dragTargetDragEnter. Should that be possible?
If so, can you refactor the code to make that more clear?

[paulmeyer_, 2017-02-16 19:30:30]

On 2017/02/15 18:11:35, pdr. wrote:
> On 2017/02/14 at 15:53:17, paulmeyer wrote:
> > 
> 
> I think this is the bug, but we might be able to reason about this crash and
> make a real fix. It looks like we're somehow getting into the
dragTargetDragOver
> state without first going through dragTargetDragEnter. Should that be
possible?
> If so, can you refactor the code to make that more clear?

It is not meant to be possible for drag over to occur before drag enter, and I'm
not sure yet how this is happening. I haven't actually been able to repro this
crash locally yet. What I was hoping to do is land this patch which will at
least prevent the crash and unblock stable. I can then continue looking into
figuring out exactly how this is happening and try to fix the underlying cause.

[dcheng, 2017-02-16 19:47:20]

On 2017/02/16 19:30:30, paulmeyer_ wrote:
> On 2017/02/15 18:11:35, pdr. wrote:
> > On 2017/02/14 at 15:53:17, paulmeyer wrote:
> > > 
> > 
> > I think this is the bug, but we might be able to reason about this crash and
> > make a real fix. It looks like we're somehow getting into the
> dragTargetDragOver
> > state without first going through dragTargetDragEnter. Should that be
> possible?
> > If so, can you refactor the code to make that more clear?
> 
> It is not meant to be possible for drag over to occur before drag enter, and
I'm
> not sure yet how this is happening. I haven't actually been able to repro this
> crash locally yet. What I was hoping to do is land this patch which will at
> least prevent the crash and unblock stable. I can then continue looking into
> figuring out exactly how this is happening and try to fix the underlying
cause.

Please add a TODO to figure out the real fix. Drag and drop is already pretty
messy, so it's important to have a tracking followup to understand why the
invariants the embedder (Aura/content) should be providing aren't holding.

[paulmeyer, 2017-02-16 20:36:15]

On 2017/02/16 19:47:20, dcheng wrote:
> On 2017/02/16 19:30:30, paulmeyer_ wrote:
> > On 2017/02/15 18:11:35, pdr. wrote:
> > > On 2017/02/14 at 15:53:17, paulmeyer wrote:
> > > > 
> > > 
> > > I think this is the bug, but we might be able to reason about this crash
and
> > > make a real fix. It looks like we're somehow getting into the
> > dragTargetDragOver
> > > state without first going through dragTargetDragEnter. Should that be
> > possible?
> > > If so, can you refactor the code to make that more clear?
> > 
> > It is not meant to be possible for drag over to occur before drag enter, and
> I'm
> > not sure yet how this is happening. I haven't actually been able to repro
this
> > crash locally yet. What I was hoping to do is land this patch which will at
> > least prevent the crash and unblock stable. I can then continue looking into
> > figuring out exactly how this is happening and try to fix the underlying
> cause.
> 
> Please add a TODO to figure out the real fix. Drag and drop is already pretty
> messy, so it's important to have a tracking followup to understand why the
> invariants the embedder (Aura/content) should be providing aren't holding.

Added.

[pdr., 2017-02-16 20:39:55]

On 2017/02/16 at 20:36:15, paulmeyer wrote:
> On 2017/02/16 19:47:20, dcheng wrote:
> > On 2017/02/16 19:30:30, paulmeyer_ wrote:
> > > On 2017/02/15 18:11:35, pdr. wrote:
> > > > On 2017/02/14 at 15:53:17, paulmeyer wrote:
> > > > > 
> > > > 
> > > > I think this is the bug, but we might be able to reason about this crash
and
> > > > make a real fix. It looks like we're somehow getting into the
> > > dragTargetDragOver
> > > > state without first going through dragTargetDragEnter. Should that be
> > > possible?
> > > > If so, can you refactor the code to make that more clear?
> > > 
> > > It is not meant to be possible for drag over to occur before drag enter,
and
> > I'm
> > > not sure yet how this is happening. I haven't actually been able to repro
this
> > > crash locally yet. What I was hoping to do is land this patch which will
at
> > > least prevent the crash and unblock stable. I can then continue looking
into
> > > figuring out exactly how this is happening and try to fix the underlying
> > cause.
> > 
> > Please add a TODO to figure out the real fix. Drag and drop is already
pretty
> > messy, so it's important to have a tracking followup to understand why the
> > invariants the embedder (Aura/content) should be providing aren't holding.
> 
> Added.

Thanks, LGTM

[paulmeyer, 2017-02-16 20:43:15]

The CQ bit was checked by paulmeyer@chromium.org

[commit-bot: I haz the power, 2017-02-16 20:44:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 00:13:01]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487277795999530,
"parent_rev": "0a8d26e24fbf3e0a262f2e0fe72100c979a77428", "commit_rev":
"1b7963b7210d4d1a45049841cf83eb30274ba82d"}

[commit-bot: I haz the power, 2017-02-17 00:13:34]

Description was changed from

==========
Speculative fix for crash in
blink::WebFrameWidgetBase::dragTargetDragEnterOrOver.

BUG=671504
==========

to

==========
Speculative fix for crash in
blink::WebFrameWidgetBase::dragTargetDragEnterOrOver.

BUG=671504

Review-Url: https://codereview.chromium.org/2695043002
Cr-Commit-Position: refs/heads/master@{#451149}
Committed:
https://chromium.googlesource.com/chromium/src/+/1b7963b7210d4d1a45049841cf83...
==========

[commit-bot: I haz the power, 2017-02-17 00:13:35]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/1b7963b7210d4d1a45049841cf83...